[FFmpeg-devel] [PATCH v1] avcodec/av1dec: Add tile list OBU to decompose list

[Andreas Rheinhardt]

Wang, Fei W:
> On Thu, 2023-12-21 at 20:14 +0100, Andreas Rheinhardt wrote:
>> Wang, Fei W:
>>> On Wed, 2023-12-20 at 17:11 +0100, Andreas Rheinhardt wrote:
>>>> fei.w.wang-at-intel.com at ffmpeg.org:
>>>>> From: Fei Wang <fei.w.wang at intel.com>
>>>>>
>>>>> Show the unsupported message and return unsupported for clips
>>>>> contain
>>>>> tile list OBU since it hasn't been implemented. Otherwise,
>>>>> decoding
>>>>> maybe successful but result is incorrect.
>>>>>
>>>>> Signed-off-by: Fei Wang <fei.w.wang at intel.com>
>>>>> ---
>>>>>  libavcodec/av1dec.c | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>>>> index 4dcde234c6..629e37c3f8 100644
>>>>> --- a/libavcodec/av1dec.c
>>>>> +++ b/libavcodec/av1dec.c
>>>>> @@ -805,6 +805,7 @@ static const CodedBitstreamUnitType
>>>>> decompose_unit_types[] = {
>>>>>      AV1_OBU_SEQUENCE_HEADER,
>>>>>      AV1_OBU_TEMPORAL_DELIMITER,
>>>>>      AV1_OBU_TILE_GROUP,
>>>>> +    AV1_OBU_TILE_LIST,
>>>>
>>>> What do you need this for? Decomposing it would only change
>>>> whether
>>>> CodedBitstreamUnit.content is available, but you are only reading
>>>> CodedBitstreamUnit.type.
>>>
>>> To show the unsupported error and let user know that there are tile
>>> list OBUs in bitstream that decoder can't handle them. Otherwise,
>>> like
>>> my commit mentioned, tile list obu bitsteam may be decoded
>>> 'successful'
>>> according to log.
>>>
>>
>> As I said: You do not need CodedBitstreamUnit.content for the error
>> message.
> 
> No, obu will be filtered if it is not listed in decompose, decoder
> can't receive its type neither.
> 

Did you test this? If so, this would be a violation of the documentation
of decompose_unit_types: "Types not in this list will be available in
bitstream form only." My reading of the code of
cbs_read_fragment_content() in cbs.c is that it does what it is supposed
to do.

If you were right, it would also mean that the current case for
AV1_OBU_TILE_LIST and AV1_OBU_PADDING is redundant (and that the default
case will be used instead).

- Andreas