[FFmpeg-devel] [PATCH 14/18] h264_ps: make the PPS hold a reference to its SPS

Anton Khirnov anton at khirnov.net
Mon Mar 23 19:36:44 EET 2020


Quoting James Almer (2020-03-18 17:05:38)
> On 3/13/2020 7:28 AM, Anton Khirnov wrote:
> > It represents the relationship between them more naturally and will be
> > useful in the following commits.
> > 
> > Allows significantly more frames in fate-h264-attachment-631 to be
> > decoded.
> 
> That sample is odd. It has an SPS/PPS pair in extradata that's wrong for
> the Buffering Period and Picture Timing SEI messages in-band, but
> seemingly correct to actually decode all these frames revealed by this
> patch.
> The SPS that makes the aforementioned SEI messages parseable, but the
> frames seemingly undecodeable, is in-band in the second RAP (Which fwiw
> reports a different bitstream level). So this patch prevents the
> extradata PPS from using the in-band SPS after it replaced the extradata
> one in sps_list. There's no PPS in-band.
> 
> Is this really intended? Is the active PPS' sps_id meant to reference
> the SPS with that id that's currently "valid" and in sps_list, or the
> whichever one was valid at the time the PPS was first parsed?. If the
> latter, then why replace the SPS in-band but never the PPS?
> The in-band SPS is virtually unused after this change.

My guess would be that someone (or some/thing/, dun dun dun) did some
botched/misunderstood processing on that sample, so trying to figure out
what was intended is futile.

According to the spec, SPS contents must not change within a sequence
(they can change on sequence boundaries), so this sample is invalid.
>From a more practical viewpoint, since a PPS is parsed for a given SPS
I'd say it is clearer, cleaner and safer to tie them together like I'm
doing in this patch.

> 
> > ---
> >  libavcodec/h264_ps.c               |  29 +++++-
> >  libavcodec/h264_ps.h               |   3 +
> >  libavcodec/h264_slice.c            |  14 +--
> 
> Missing changes to h264_parser.c, and one "h->ps.sps == (const
> SPS*)h->ps.sps_list[h->ps.pps->sps_id]->data" check in h264dec.c

Good catch, fixed locally.

-- 
Anton Khirnov


More information about the ffmpeg-devel mailing list