On 29/04/2026 09:22, 牟凡 via ffmpeg-devel wrote:
> Thanks for the LGTM.
>
>
>> When you say it "desynchronises the parser", what downstream issues
>> were you observing: mismatches or crashes?
>
>
> Hard parse failure, neither crashes nor silent mismatches.
>
>
> The PH parser ends up reading two extra ue(v) syntax elements (or
> skipping two it should have read), depending on whether SPS and PH
> disagree on chroma MTT depth being zero. By the time
> picture_header_structure() reaches its trailing byte_alignment_bit,
> the bit position is off, the sanity check
> "byte_alignment_bit_equal_to_one out of range: 0, but must be in
> [1,1]" trips, and the PH NAL unit returns AVERROR_INVALIDDATA.
>
>
> cbs_h266 is wired into vvc_parser.c via "#include cbs_h266.h" and
> is invoked on every VVC_PH_NUT, so the failure is not limited to bsf
> consumers; it surfaces in the regular VVC decode path as
> "Failed to parse picture unit", and from there every following slice
> fails with "PPS id 0 not available" / "Failed to read packet".
>
>
> Concrete numbers from the 4136-frame compliant 720p sample attached
> to issue 22958:
>
>
> pre-fix: 4127 packets read, 0 frames decoded, 4127 decode errors;
> ffmpeg exits 69 ("Conversion failed!")
> post-fix: 4136 packets read, 4136 frames decoded, 0 decode errors;
> ffmpeg exits 0.
>
>
> So the user-visible effect is "an entirely undecodable stream with a
> clean error message", not partial corruption. A parser without the
> trailing alignment-bit sanity check could in principle desync further
> into the slice headers and produce silent garbage, but in our case
> that check stops the slide cleanly.
>
>
>> Do we need to backport this?
>
>
> I would say yes, to every active release branch.
>
>
> The buggy line was introduced in dfc62fd1c6da6429bbd0eb3cbb6f3804e8fcb8ae
> ("avcodec/cbs: add cbs implementation for H266/VVC", 2023-03-21) and
> has not been touched since, so 6.x / 7.x / 8.0 / 8.1 release branches
> all carry it identically. The trigger is admittedly narrow
> (sps_qtbtt_dual_tree_intra_flag = 1 plus a partition_constraints
> override that flips chroma MTT depth across zero), but when it does
> trigger the stream is wholly undecodable on stock ffmpeg, which for
> downstream OTT / broadcast users is effectively a hard "compliant
> stream X cannot be decoded" rather than a quality regression.
>
>
> Happy to send rebased one-liners to the active release branches if
> that is the preferred path; just let me know which branches you want
> covered.
>
>
>
>
>
>
>
>
>
>
>
> At 2026-04-29 16:06:28, "Frank Plowman via ffmpeg-devel"
> <[email protected]> wrote:
>> On 29/04/2026 08:17, Mou Fan via ffmpeg-devel wrote:
>>> In the picture header parser, the chroma branch incorrectly tested
>>> sps_max_mtt_hierarchy_depth_intra_slice_chroma to decide whether to
>>> parse ph_log2_diff_max_{bt,tt}_min_qt_intra_slice_chroma.
>>>
>>> Per ITU-T H.266 (V4, 01/2026) section 7.3.2.8 "Picture header
>>> structure syntax", the condition is on the just-parsed
>>> ph_max_mtt_hierarchy_depth_intra_slice_chroma, exactly mirroring the
>>> luma branch a few lines above and the inter-slice branch below.
>>> sps_partition_constraints_override_enabled_flag allows the picture
>>> header to override the SPS values, so testing the SPS field is
>>> incorrect and desynchronises the parser whenever the PH override
>>> changes the chroma MTT depth from/to zero.
>>>
>>> Signed-off-by: Mou Fan <[email protected]>
>>> ---
>>> libavcodec/cbs_h266_syntax_template.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/cbs_h266_syntax_template.c
>>> b/libavcodec/cbs_h266_syntax_template.c
>>> index 98a8954943..fc79ba46c7 100644
>>> --- a/libavcodec/cbs_h266_syntax_template.c
>>> +++ b/libavcodec/cbs_h266_syntax_template.c
>>> @@ -2819,7 +2819,7 @@ static int FUNC(picture_header)
>>> (CodedBitstreamContext *ctx, RWContext *rw,
>>> 0, FFMIN(6, ctb_log2_size_y) - min_cb_log2_size_y);
>>> ue(ph_max_mtt_hierarchy_depth_intra_slice_chroma,
>>> 0, 2 * (ctb_log2_size_y - min_cb_log2_size_y));
>>> - if (sps->sps_max_mtt_hierarchy_depth_intra_slice_chroma !=
>>> 0) {
>>> + if (current->ph_max_mtt_hierarchy_depth_intra_slice_chroma
>>> != 0) {
>>> unsigned int min_qt_log2_size_intra_c =
>>>
>>> current->ph_log2_diff_min_qt_min_cb_intra_slice_chroma +
>>> min_cb_log2_size_y;
>>
>> LGTM.
>>
>> When you say it "desynchronises the parser", what downstream issues were
>> you observing: mismatches or crashes? Do we need to backport this?
>>
>> --
>> Frank
> _______________________________________________
> ffmpeg-devel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]Pushed, thanks. -- Frank
OpenPGP_0x03A84C6A098F2C6B.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
