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). 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".
