On Sun, Jun 16, 2024 at 12:38 AM Frank Plowman <p...@frankplowman.com> wrote:
> n 15/06/2024 13:24, Nuo Mi wrote: > > On Sat, Jun 15, 2024 at 2:35 PM Christophe Gisquet < > > christophe.gisq...@gmail.com> wrote: > > > >> Le ven. 14 juin 2024, 11:39, Frank Plowman <p...@frankplowman.com> a > >> écrit : > >> > >>> When the SPS associated with a particular SPS ID changes, invalidate > all > >>> the PPSs which use that SPS ID. Fixes crashes with illegal bitstreams. > >>> This is done in the CBS, rather than in libavcodec/vvc/ps.c like the > SPS > >>> ID reuse validation, as parts of the CBS parsing process for PPSs > >>> depend on the SPS being referred to. > >>> > >> > >> I am uncertain about this. I have no definite knowledge nor proof, but I > >> would have thought these are persistent, IE it's legal to update some of > >> them, their validity depending on something else. > >> > > > >> Wondering if the tested streams are thus conformant. > >> > >> But I don't know the actual rule. Maybe finding an EOB/EOS NUT? Related > to > >> some particular shape of a clean random access point, that would require > >> retransmitting VPS/SPS/PPS/APS/... ? > >> > >> Asking Benjamin Bross might be a better option here. > >> > > Hi Chris, > > spec said sps should not change in a CVS. Frank has some patches to fix > a > > similar issue. > > > https://github.com/FFmpeg/FFmpeg/commit/2d79ae3f8a3306d24afe43ba505693a8dbefd21b > > > > > > Hi Frank, > > Did it crash before your error hand code in ps.c? > > Could you send me the clip? > > > > Thank you > > > > Hi both, > > Thank you for your reviews. > > An example of a crashing bitstream which is fixed by this patch is ID > 295 available here: https://github.com/ffvvc/tests/pull/43. The > relevant part of the bitstream is a sequence of NAL units > > AU (decode_order=5) > 18. SPS > sps_seq_parameter_set_id = 0 > sps_ctb_log2_size_y = 5 > 19. PPS > pps_pic_parameter_set_id = 0 > pps_seq_parameter_set_id = 0 > 20. IDR_N_LP > ph_pic_order_cnt_lsb = 0 > NoOutputBeforeRecoveryFlag = 1 > ph_pic_parameter_set_id = 0 > > AU (decode_order=6) > 21. AUD > 22. VPS > 23. SPS > sps_seq_parameter_set_id = 0 > sps_ctb_log2_size_y = 7 > 24. PREFIX_APS > 25. IDR_N_LP > ph_pic_order_cnt_lsb = 0 > NoOutputBeforeRecoveryFlag = 1 > ph_pic_parameter_set_id = 0 > > The layout of SPSs alone is legal (not covered by the checks introduced > in 2d79ae3f8a3306d24afe43ba505693a8dbefd21b) as the second AU is a CLVSS > AU. As a result, the bitstream crashes both before and after > 2d79ae3f8a3306d24afe43ba505693a8dbefd21b. What this patch does is > produce an error when the VCL NAL unit in the second AU (25.) tries to > use PPS ID 0, as the SPS NAL unit that PPS was defined with reference to > (18.) is no longer available. > > Christophe, is my interpretation of your point correct when I say you > are suggesting that the above sequence may be legal, so long as the PPS > still satisfies the new bounds etc. derived from the second SPS? I did > consider this, and I think it may be possible to implement by delaying > CBS element validation and inference until libavcodec/vvc/ps.c. > However, there are no bitstreams in the conformance suite which contain > such a structure and this is different to how the native HEVC decoder > behaves (see libavcodec/hevc/ps.c:72). > Pretty solid analysis, thank you, Frank > > All the best, > -- > Frank > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
