[FFmpeg-devel] [PATCH] ffmpeg: pass the correct AVCodecContext to av_parser_change()

[James Almer]

On 2/23/2018 4:35 AM, Hendrik Leppkes wrote:
> On Fri, Feb 23, 2018 at 1:28 AM, James Almer <jamrial at gmail.com> wrote:
>> On 2/22/2018 8:52 PM, Hendrik Leppkes wrote:
>>> On Fri, Feb 23, 2018 at 12:37 AM, James Almer <jamrial at gmail.com> wrote:
>>>> On 2/22/2018 8:34 PM, Hendrik Leppkes wrote:
>>>>> On Fri, Feb 23, 2018 at 12:20 AM, James Almer <jamrial at gmail.com> wrote:
>>>>>> av_parser_change() is effectively a noop if the avctx passed it to
>>>>>> doesn't have the global header or local header flags set, and
>>>>>> initializing a custom AVCodecContext as a copy of an
>>>>>> AVCodecParameters results in the flags and flags2 fields being zero.
>>>>>> Use instead the existing custom AVCodecContext with the required
>>>>>> flags set in ffmpeg_opt.c new_output_stream() among other places.
>>>>>>
>>>>>> The fate test change is the result of the hevc sps/pps/vps being
>>>>>> removed from frames when copying into a format that explicitly
>>>>>> states it uses global header.
>>>>>>
>>>>>
>>>>> Deleting inband headers is not necessarily a good idea, and likely
>>>>> also why its being skipped for h264.
>>>>
>>>> Well, I'd argue it is when the user and/or the output format request it.
>>>>
>>>
>>> Unless there is an explicit option to request that, thats not exactly the case.
>>>
>>> - Hendrik
>>
>> The AVCodecContext global_header flag, which is what av_parser_change()
>> looks at. It's either set with an AVOption (With the CLI that's using
>> -flags +global_header) when encoding, or force enabled in
>>
>> https://git.videolan.org/?p=ffmpeg.git;a=blob;f=fftools/ffmpeg_opt.c;h=997d53838108c2c3deb7fa3d3f395712738dad86;hb=HEAD#l1495
>>
>> as mentioned in the commit message, depending on the constant
>> flag/capability set by the muxer.
> 
> Thats not what I mean, I know about that flag - and it controls AnnexB
> vs MP4, but it does not explicitly say "remove inband headers as
> well".
> If anything I think HEVC should follow the H264 example for consistency.
> 
> There are plenty cases where removing the in-band headers results in problems.

So, if removing in-band headers is not desired, what exactly was the
point/benefit of this entire chunk of code? I'm not sure the
local_header portion of av_parser_change (to dump out of band headers at
every keyframe) can be triggered currently. Neither -flags
+global_header or -flags2 +local_header seem to work for codec copy,
seeing the former is ignored and the latter gives an error.

Who wrote it, for that matter? It's been dead code for years.
Maybe it should be removed altogether. Nobody missed it all this time,
and skipping hevc as well would leave mpeg4 only.