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

Michael Niedermayer michael at niedermayer.cc
Sat Feb 24 02:13:45 EET 2018


On Fri, Feb 23, 2018 at 11:15:50AM -0300, James Almer wrote:
> 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.

I suspect (but someone would have to check the various specs to confirm)
that the in band headers are not really correct in the cases where a place
for global headers exist for these headers. Even if they are correct they 
are a waste of space as they must be repeated at _every_ point where 
seeking can bring one to.

The "correct" thing to do is to collect all the in band headers, put them
ALL in the global header spot of the container and update the index in band
to refer to them. Or use other higher level features to switch between global
headers where the container supports this. 

This is much harder than just leaving the headers in place in band of course


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180224/ba3735c6/attachment.sig>


More information about the ffmpeg-devel mailing list