[FFmpeg-devel] [PATCH] lavc/vvc: Add check to num_multi_layer_olss

[Frank Plowman]

On 30/01/2024 12:55, Frank Plowman wrote:
> On 30/01/2024 12:31, Nuo Mi wrote:
> 
>> On Tue, Jan 30, 2024 at 5:41 PM<post at frankplowman.com>  wrote:
>>> From: Frank Plowman<post at frankplowman.com>
>>>
>>> Check that vps_each_layer_is_an_ols_flag, which indicates that "at
>>> least one OLS specified by the VPS contains more than one layer," is
>>> set if num_multi_layer_olss is non-zero.
>>>
>>> Fixes:
>>> 65160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-4665241535119360
>>>
>>> Found-by: continuous fuzzing process
>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by
>>> <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
>>> Frank Plowman<post at frankplowman.com>
>>> ---
>>>   libavcodec/cbs_h266_syntax_template.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavcodec/cbs_h266_syntax_template.c
>>> b/libavcodec/cbs_h266_syntax_template.c
>>> index 2f3478e5e1..37dc3acba0 100644
>>> --- a/libavcodec/cbs_h266_syntax_template.c
>>> +++ b/libavcodec/cbs_h266_syntax_template.c
>>> @@ -911,6 +911,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
>>> RWContext *rw,
>>>                   num_multi_layer_olss++;
>>>               }
>>>           }
>>> +        if (!current->vps_each_layer_is_an_ols_flag &&
>>> num_multi_layer_olss == 0)
>>> +            return AVERROR_INVALIDDATA;
>>>       }
>> The specification does not provide information on how to obtain
>> TotalNumOlss (total_num_olss) when ols_mode_idc is set to 3.
>> Therefore, the earlier line "u(8, vps_num_ptls_minus1, 0, 
>> total_num_olss -
>> 1)" is undefined.
>> We'd better return a patch welcome error instead of printing a warning
>> before vps_num_ptls_minus1 line
> 
> This is the same behaviour James suggested in an earlier patch. The spec 
> says "decoders conforming to this version of this Specification shall 
> ignore the OLSs with vps_ols_mode_idc equal to 3." I don't think this 
> should be an error as the spec is unambiguous here. Perhaps we can 
> instead skip the remainder of the VPS if vps_ols_mode_idc is 3? Or is 
> there some better way to ignore these OLSs?

For reference, VTM's behaviour is the same as the current behaviour: 
TotalNumOlss is assumed to be 0 when ols_mode_idc, hence most of the 
remaining syntax elements in the VPS are not read as they are within

for (i = 0; i < total_num_olss; i++)

loops or other loops with bounds derived from total_num_olss.  On the 
other hand, VVdeC's behaviour is the same as you suggest: it throws an 
error if total_num_olss is 3.