[FFmpeg-devel] [PATCH] avformat/matroskaenc: flag discardable packets as such

[James Almer]

On 12/15/2017 2:56 PM, John Stebbins wrote:
> On 12/13/2017 07:14 PM, James Almer wrote:
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavformat/matroskaenc.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> This only has an effect when muxing h265 streams originating from the
>> libx265 wrapper atm, as no other encoder or demuxer currently sets
>> the flag.
>>
>> I compared the output of our muxer with the latest mkvmerge, and in
>> the latter a few more SimpleBlocks were flagged as discardable than
>> by our muxer after this patch.
>> I can't say if our libx265 wrapper is not properly flagging what it
>> should, or if mkvmerge's parser is flagging more frames than it
>> should.
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index f22c2ab70c..915ef3c107 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -2100,7 +2100,8 @@ static void mkv_write_block(AVFormatContext *s, AVIOContext *pb,
>>      MatroskaMuxContext *mkv = s->priv_data;
>>      AVCodecParameters *par = s->streams[pkt->stream_index]->codecpar;
>>      uint8_t *data = NULL, *side_data = NULL;
>> -    int offset = 0, size = pkt->size, side_data_size = 0;
>> +    const int discardable = !!(pkt->flags & AV_PKT_FLAG_DISPOSABLE);
>> +    int offset = 0, size = pkt->size, side_data_size = 0, flags = 0;
>>      int64_t ts = mkv->tracks[pkt->stream_index].write_dts ? pkt->dts : pkt->pts;
>>      uint64_t additional_id = 0;
>>      int64_t discard_padding = 0;
>> @@ -2160,12 +2161,15 @@ static void mkv_write_block(AVFormatContext *s, AVIOContext *pb,
>>          blockid = MATROSKA_ID_BLOCK;
>>      }
>>  
>> +    if (blockid == MATROSKA_ID_SIMPLEBLOCK)
>> +        flags = (keyframe << 7) | discardable;
>> +
>>      put_ebml_id(pb, blockid);
>>      put_ebml_num(pb, size + 4, 0);
>>      // this assumes stream_index is less than 126
>>      avio_w8(pb, 0x80 | track_number);
>>      avio_wb16(pb, ts - mkv->cluster_pts);
>> -    avio_w8(pb, (blockid == MATROSKA_ID_SIMPLEBLOCK && keyframe) ? (1 << 7) : 0);
>> +    avio_w8(pb, flags);
>>      avio_write(pb, data + offset, size);
>>      if (data != pkt->data)
>>          av_free(data);
> 
> LGTM. 
> 
> FYI, I have a pending patch that does the same thing (in a slightly different way) as well as a patch for reading the discardable flag from mkv files.

Ah, should have known that'd be the case seeing the flag was just
introduced.

  But I wanted to wait till my other patches related to the discardable
flag were fully reviewed and
> accepted before adding more to the list.  As a reminder, there is still a patch to set discardable flag in the x264 encoder, a patch to read the flag in mp4 and a patch to use the flag in ffplay that are not yet accepted, though there are no unresolved
> comments for these patch submissions.

Do you have any idea why the output of matroskaenc differs from that of
mkvmerge after this patch? Does libx265 (or the avcodec wrapper) not
flag all B frames as disposable, or is mkvmerge flagging the wrong frames?