[FFmpeg-devel] [PATCH 2/5] avcodec/hevc/hevcdec: Do not allow changes to parameter sets between slices

[Anton Khirnov]

Quoting Michael Niedermayer (2024-06-26 01:27:36)
> The specification explcitly doesnt allow the active *PS to change
> 
> "Any PPS NAL unit containing the value of pps_pic_parameter_set_id for the active PPS RBSP for a coded picture (and
> consequently for the layer containing the coded picture) shall have the same content as that of the active PPS RBSP for the
> coded picture, unless it follows the last VCL NAL unit of the coded picture and precedes the first VCL NAL unit of another
> coded picture."
> 
> "Any SPS NAL unit with nuh_layer_id equal to 0 containing the value of sps_seq_parameter_set_id for the active SPS
> RBSP for the base layer for a CVS shall have the same content as that of the active SPS RBSP for the base layer for the
> CVS, unless it follows the last access unit of the CVS and precedes the first VCL NAL unit and the first SEI NAL unit
> containing an active parameter sets SEI message (when present) of another CVS."
> 
> "Any SPS NAL unit with any nuh_layer_id value containing the value of sps_seq_parameter_set_id for the active SPS
> RBSP for a particular layer shall have the same content as that of the active SPS RBSP for the particular layer unless it
> follows the access unit auA containing the last coded picture for which the active SPS RBSP for the particular layer is
> required to be active for the particular layer and precedes the first NAL unit succeeding auA in decoding order that activates
> an SPS RBSP with the same value of seq_parameter_set_id."

I have no idea why you're quoting the spec at me, I know perfectly well
the PPS cannot change between slices. My point is this: if the current
check for PPS change does not detect it, it should be fixed.
Specifically, it should compare actual PPS objects rather than pps_id.

> So if anything fancy is wanted, the way to go is not to parse it in the first place.
> Because if it matches then the parsing was a waste of time. If it mismatches, that
> would be invalid and would just lead to failure anyway
> 
> But really the primary goal is to fix the out of array access and be easy to
> backport. I am not trying to fix this in a fancy way.
> The simpler and more robust the fix is the better.

This "simple and robust" approach of sprinkling random checks all over
the place leads to unreadable and unmaintainable code.

-- 
Anton Khirnov