[FFmpeg-devel] [PATCH 10/21] avformat/matroskadec: Don't keep old blocks
Steve Lhomme
robux4 at ycbcr.xyz
Sun Apr 7 10:05:44 EEST 2019
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
> Before this commit, the Matroska muxer would read a block when required
> to do so, parse the block, create and return the necessary AVPackets and
> yet keep the blocks (in a dynamically allocated list), although they
> aren't used at all any more. This has been changed. There is no list any
> more and the block is immediately discarded after parsing.
My understanding of the code is that the blocks are put in a queue,
that's whatwebm_clusters_start_with_keyframe() uses to check that the
first frame is a keyframe (it doesn't check the type of the frame
though...). But since there's only one read
inmatroska_parse_cluster_incremental()there's only one kept in memory.
So LGTM.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at googlemail.com>
> ---
> libavformat/matroskadec.c | 87 +++++++++++++++++----------------------
> 1 file changed, 38 insertions(+), 49 deletions(-)
>
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 9198fa1bea..997c96d78f 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -304,9 +304,20 @@ typedef struct MatroskaLevel {
> uint64_t length;
> } MatroskaLevel;
>
> +typedef struct MatroskaBlock {
> + uint64_t duration;
> + int64_t reference;
> + uint64_t non_simple;
> + EbmlBin bin;
> + uint64_t additional_id;
> + EbmlBin additional;
> + int64_t discard_padding;
> +} MatroskaBlock;
> +
> typedef struct MatroskaCluster {
> + MatroskaBlock block;
> uint64_t timecode;
> - EbmlList blocks;
> + int64_t pos;
> } MatroskaCluster;
>
> typedef struct MatroskaLevel1Element {
> @@ -356,8 +367,6 @@ typedef struct MatroskaDemuxContext {
> MatroskaLevel1Element level1_elems[64];
> int num_level1_elems;
>
> - int current_cluster_num_blocks;
> - int64_t current_cluster_pos;
> MatroskaCluster current_cluster;
>
> /* WebM DASH Manifest live flag */
> @@ -367,16 +376,6 @@ typedef struct MatroskaDemuxContext {
> int bandwidth;
> } MatroskaDemuxContext;
>
> -typedef struct MatroskaBlock {
> - uint64_t duration;
> - int64_t reference;
> - uint64_t non_simple;
> - EbmlBin bin;
> - uint64_t additional_id;
> - EbmlBin additional;
> - int64_t discard_padding;
> -} MatroskaBlock;
> -
> static const EbmlSyntax ebml_header[] = {
> { EBML_ID_EBMLREADVERSION, EBML_UINT, 0, offsetof(Ebml, version), { .u = EBML_VERSION } },
> { EBML_ID_EBMLMAXSIZELENGTH, EBML_UINT, 0, offsetof(Ebml, max_size), { .u = 8 } },
> @@ -705,9 +704,9 @@ static const EbmlSyntax matroska_blockgroup[] = {
> };
>
> static const EbmlSyntax matroska_cluster_parsing[] = {
> - { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster, timecode) },
> - { MATROSKA_ID_BLOCKGROUP, EBML_NEST, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
> - { MATROSKA_ID_SIMPLEBLOCK, EBML_PASS, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
> + { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster, timecode) },
> + { MATROSKA_ID_BLOCKGROUP, EBML_NEST, 0, 0, { .n = matroska_blockgroup } },
> + { MATROSKA_ID_SIMPLEBLOCK, EBML_PASS, 0, 0, { .n = matroska_blockgroup } },
> { MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
> { MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
> { MATROSKA_ID_INFO, EBML_NONE },
> @@ -3443,57 +3442,48 @@ end:
>
> static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
> {
> - EbmlList *blocks_list;
> - MatroskaBlock *blocks;
> - int i, res;
> + MatroskaCluster *cluster = &matroska->current_cluster;
> + MatroskaBlock *block = &cluster->block;
> + int res;
> res = ebml_parse(matroska,
> matroska_cluster_parsing,
> - &matroska->current_cluster);
> + cluster);
> if (res == 1) {
> /* New Cluster */
> - if (matroska->current_cluster_pos)
> + if (cluster->pos)
> ebml_level_end(matroska);
> - ebml_free(matroska_cluster_parsing, &matroska->current_cluster);
> - memset(&matroska->current_cluster, 0, sizeof(MatroskaCluster));
> - matroska->current_cluster_num_blocks = 0;
> - matroska->current_cluster_pos = avio_tell(matroska->ctx->pb);
> + cluster->pos = avio_tell(matroska->ctx->pb);
> /* sizeof the ID which was already read */
> if (matroska->current_id)
> - matroska->current_cluster_pos -= 4;
> + cluster->pos -= 4;
> res = ebml_parse(matroska,
> matroska_clusters,
> - &matroska->current_cluster);
> + cluster);
> /* Try parsing the block again. */
> if (res == 1)
> res = ebml_parse(matroska,
> matroska_cluster_parsing,
> - &matroska->current_cluster);
> + cluster);
> }
>
> - if (!res &&
> - matroska->current_cluster_num_blocks <
> - matroska->current_cluster.blocks.nb_elem) {
> - blocks_list = &matroska->current_cluster.blocks;
> - blocks = blocks_list->elem;
> + if (!res && block->bin.size > 0) {
> + int is_keyframe = block->non_simple ? block->reference == INT64_MIN : -1;
> + uint8_t* additional = block->additional.size > 0 ?
> + block->additional.data : NULL;
>
> - matroska->current_cluster_num_blocks = blocks_list->nb_elem;
> - i = blocks_list->nb_elem - 1;
> - if (blocks[i].bin.size > 0 && blocks[i].bin.data) {
> - int is_keyframe = blocks[i].non_simple ? blocks[i].reference == INT64_MIN : -1;
> - uint8_t* additional = blocks[i].additional.size > 0 ?
> - blocks[i].additional.data : NULL;
> -
> - res = matroska_parse_block(matroska, blocks[i].bin.buf, blocks[i].bin.data,
> - blocks[i].bin.size, blocks[i].bin.pos,
> + res = matroska_parse_block(matroska, block->bin.buf, block->bin.data,
> + block->bin.size, block->bin.pos,
> matroska->current_cluster.timecode,
> - blocks[i].duration, is_keyframe,
> - additional, blocks[i].additional_id,
> - blocks[i].additional.size,
> - matroska->current_cluster_pos,
> - blocks[i].discard_padding);
> - }
> + block->duration, is_keyframe,
> + additional, block->additional_id,
> + block->additional.size,
> + cluster->pos,
> + block->discard_padding);
> }
>
> + ebml_free(matroska_blockgroup, block);
> + memset(block, 0, sizeof(*block));
> +
> return res;
> }
>
> @@ -3591,7 +3581,6 @@ static int matroska_read_close(AVFormatContext *s)
> for (n = 0; n < matroska->tracks.nb_elem; n++)
> if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)
> av_freep(&tracks[n].audio.buf);
> - ebml_free(matroska_cluster_parsing, &matroska->current_cluster);
> ebml_free(matroska_segment, matroska);
>
> return 0;
> --
> 2.19.2
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list