Steve Lhomme:
> 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,
Yes, the parsed blocks are put in a queue of their own; so we don't
need to keep all the raw data of all the already parsed blocks of the
current cluster around. (Refcounting means that some of this data
might be keep a little longer though, but even in this case this patch
eliminates e.g. the constant reallocation of the list of blocks.)
> that's whatwebm_clusters_start_with_keyframe() uses to check that the
> first frame is a keyframe
Yes.
> (it doesn't check the type of the frame though...).
I see a "if (!(pkt->flags & AV_PKT_FLAG_KEY))" in there.
> 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.rheinha...@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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
