[FFmpeg-devel] [PATCH v1] avformat/mux: Set AV_PKT_FLAG_KEY for is_intra_only packet

[Andreas Rheinhardt]

Limin Wang:
> On Fri, Apr 03, 2020 at 11:23:59PM +0200, Michael Niedermayer wrote:
>> On Fri, Apr 03, 2020 at 11:05:59PM +0800, lance.lmwang at gmail.com wrote:
>>> From: Limin Wang <lance.lmwang at gmail.com>
>>>
>>> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
>>> ---
>>>  libavformat/mux.c                | 16 ++++++++
>>>  tests/ref/fate/binsub-movtextenc |  2 +-
>>>  tests/ref/fate/movenc            | 50 +++++++++++------------
>>>  tests/ref/fate/sub2video         | 86 ++++++++++++++++++++--------------------
>>>  4 files changed, 85 insertions(+), 69 deletions(-)
>>>
>>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>>> index cc2d1e2..dc897c2 100644
>>> --- a/libavformat/mux.c
>>> +++ b/libavformat/mux.c
>>> @@ -548,6 +548,17 @@ fail:
>>>      return ret;
>>>  }
>>>  
>>> +static int is_intra_only(enum AVCodecID id)
>>> +{
>>> +    const AVCodecDescriptor *d = avcodec_descriptor_get(id);
>>> +    if (!d)
>>> +        return 0;
>>> +    if ((d->type == AVMEDIA_TYPE_VIDEO || d->type == AVMEDIA_TYPE_AUDIO) &&
>>> +        !(d->props & AV_CODEC_PROP_INTRA_ONLY))
>>> +        return 0;
>>> +    return 1;
>>> +}
>>
>> this should not be duplicated with code in utils.c
> 
> OK
> 
>>
>>
>>> +
>>>  #define AV_PKT_FLAG_UNCODED_FRAME 0x2000
>>>  
>>>  /* Note: using sizeof(AVFrame) from outside lavu is unsafe in general, but
>>> @@ -664,6 +675,11 @@ static int compute_muxer_pkt_fields(AVFormatContext *s, AVStream *st, AVPacket *
>>>          frac_add(st->internal->priv_pts, (int64_t)st->time_base.den * st->time_base.num);
>>>          break;
>>>      }
>>> +
>>> +    /* update flags */
>>> +    if (is_intra_only(st->codecpar->codec_id))
>>> +        pkt->flags |= AV_PKT_FLAG_KEY;
>>> +
>>
>> it may make sense to store the AVCodecDescriptor and not search it again for
>> each packet. But then the "is_intra_only" itself also does not change so that
>> should itself be computet just once per stream
> 
> Do you say we should add a is_intra_only field in AVStream to compute at beginning,
> however it'll change the API header. So I prefer to use is_intra_only() function first,  

This should be added to AVStreamInternal, not AVStream itself. And of
course this can and should also be used for demuxing.

- Andreas