[FFmpeg-devel] [PATCH v2] movenc: Use first H264/HEVC frame as extradata, if it is missing

[Martin Storsjö]

On Thu, 21 May 2020, Andreas Rheinhardt wrote:

> Martin Storsjö:
>> On Thu, 21 May 2020, Andreas Rheinhardt wrote:
>> 
>>> Martin Storsjö:
>>>> On Thu, 21 May 2020, Michael Niedermayer wrote:
>>>>
>>>>> On Wed, May 20, 2020 at 09:10:21AM +0300, Martin Storsjö wrote:
>>>>>> Sticking a full frame in the extradata works, as the code for writing
>>>>>> the avcC/hvcC extracts the relevant parameter set NAL units - provided
>>>>>> that they actually exist in the frame.
>>>>>>
>>>>>> Some encoders don't provide split out extradata directly on init (or
>>>>>> at all). In particular, the MediaFoundation encoder wrapper doesn't
>>>>>> always (depending on the actual encoder device) - this is the case for
>>>>>> Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
>>>>>>
>>>>>> This only works for cases where the moov hasn't already been written
>>>>>> (e.g. when not writing fragmented mp4 with empty_moov, unless using
>>>>>> the delay_moov option).
>>>>>>
>>>>>> Signed-off-by: Martin Storsjö <martin at martin.st>
>>>>>> ---
>>>>>>  libavformat/movenc.c | 4 +++-
>>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>>>>> index 27d7621e27..6a85440a3f 100644
>>>>>> --- a/libavformat/movenc.c
>>>>>> +++ b/libavformat/movenc.c
>>>>>> @@ -5584,7 +5584,9 @@ int ff_mov_write_packet(AVFormatContext *s,
>>>>>> AVPacket *pkt)
>>>>>>
>>>>>>      if ((par->codec_id == AV_CODEC_ID_DNXHD ||
>>>>>>           par->codec_id == AV_CODEC_ID_TRUEHD ||
>>>>>> -         par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len) {
>>>>>> +         par->codec_id == AV_CODEC_ID_AC3 ||
>>>>>> +         par->codec_id == AV_CODEC_ID_H264 ||
>>>>>> +         par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
>>>>>>          /* copy frame to create needed atoms */
>>>>>>          trk->vos_len  = size;
>>>>>>          trk->vos_data = av_malloc(size +
>>>>>> AV_INPUT_BUFFER_PADDING_SIZE);
>>>>>
>>>>> This changes avcintra output
>>>>>
>>>>> example testcase:
>>>>> ./ffmpeg  -i ~/videos/mm-short.mpg -avcintra-class 100 -tune psnr
>>>>> -flags +ildct-global_header -t 0.5 -pix_fmt yuv422p10 -vf
>>>>> scale=1920:1080 -an file.mov
>>>>
>>>> Right, if explicitly disabling global headers on the encoder, this will
>>>> end up putting them back.
>>>>
>>>> Any ideas on what the best path forward would be?
>>>>
>>>> // Martin
>>>
>>> If I am not mistaken, then the sample created by the above command line
>>> is annex b in mp4 which is against the spec. So changing the output is
>>> nothing to worry about.
>> 
>> No, the mov muxer does rewrite the bitstream from annex b to mp4 form,
>> regardless of whether there was any broken out parameter sets in the
>> extradata.
>> 
> Sure about that? The check used is
>
>    if (par->codec_id == AV_CODEC_ID_H264 && trk->vos_len > 0 &&
> *(uint8_t *)trk->vos_data != 1 && !TAG_IS_AVCI(trk->tag)) {
>
> If trk->vos_len is zero (which is a requirement for your patch to have
> any effect), no reformatting will be performed.

Oh, indeed, you're right.

However wrong that is from the mov/mp4 point of view, it may very well be 
that AVC intra is supposed to be written like this in mov/mp4 files - but 
we'd need a word from somebody who actually knows that case.

Hmm, and unfortunately, this check is further up in the function, than 
when creating the vos_data/vos_len from the frame, so the first frame 
doesn't end up converted correctly in my case... Will send a new patch to 
fix that.

// Martin