[FFmpeg-devel] [PATCH] lavc/vvc: AVERROR_PATCHWELCOME for subpictures

[Frank Plowman]

On 12/03/2024 03:33, Wang, Fei W wrote:
> On Mon, 2024-03-11 at 21:57 -0300, James Almer wrote:
>> On 3/11/2024 9:49 PM, Michael Niedermayer wrote:
>>> On Mon, Mar 11, 2024 at 06:53:31PM +0000, Frank Plowman wrote:
>>>> VVC's subpictures feature is not yet implemented in the native
>>>> decoder.
>>>> Throw an AVERROR_PATCHWELCOME when trying to decode a bitstream
>>>> using
>>>> the feature.  Fixes crashes when trying to decode bitstreams
>>>> which
>>>> use the feature.
>>>>
>>>> Signed-off-by: Frank Plowman <post at frankplowman.com>
>>>> ---
>>>>   libavcodec/vvc/vvc_ps.c | 15 +++++++++++++++
>>>>   1 file changed, 15 insertions(+)
>>>
>>> This breaks fate-vvc-conformance-SUBPIC_A_3
>>>
>>> make fate-vvc-conformance-SUBPIC_A_3
>>> TEST    vvc-conformance-SUBPIC_A_3
>>> --- ./tests/ref/fate/vvc-conformance-SUBPIC_A_3	2024-03-05
>>> 02:37:36.235300141 +0100
>>> +++ tests/data/fate/vvc-conformance-SUBPIC_A_3	2024-03-12
>>> 01:47:27.301593567 +0100
>>> @@ -1,9 +0,0 @@
>>> -#tb 0: 1/25
>>> -#media_type 0: video
>>> -#codec_id 0: rawvideo
>>> -#dimensions 0: 1920x1080
>>> -#sar 0: 0/1
>>> -0,          0,          0,        1,  6220800, 0xa419cfb6
>>> -0,          1,          1,        1,  6220800, 0xa419cfb6
>>> -0,          2,          2,        1,  6220800, 0xa419cfb6
>>> -0,          3,          3,        1,  6220800, 0xa419cfb6
>>> Test vvc-conformance-SUBPIC_A_3 failed. Look at
>>> tests/data/fate/vvc-conformance-SUBPIC_A_3.err for details.
>>> tests/Makefile:318: recipe for target 'fate-vvc-conformance-
>>> SUBPIC_A_3' failed
>>> make: *** [fate-vvc-conformance-SUBPIC_A_3] Error 69
>>>
>>> thx
>>
>> The sample appears to decode fine and doesn't crash, although all
>> four 
>> frames are exactly the same (I don't know is that's intended).
> 
> The result is correct. Assume native decode can handle subpic, at least
> a part of the feature.
> 
>> Maybe the crashes can be fixed in some other form? And abort only if 
>> FF_COMPLIANCE_STRICT is requested.
> 
> Previous I made a patch to fix the crash in setup ps, but still get
> error in decoding slice, it's better to return AVERROR_PATCHWELCOME
> only in case of pps_single_slice_per_subpic_flag.
> 
> https://github.com/intel-media-ci/ffmpeg/pull/723
> 
> Thanks
> Fei

Hi,

Thanks all for your reviews.

Yes, the feature is most problematic if pps_single_slice_per_subpic_flag
is 1.  This is because there is some unimplemented logic in the
parameter set parser in this case which leads to some out-of-bounds
accesses.  Subpictures will also fail to decode bitexact if
sps_independent_subpics is 1, although in practice the distortion this
introduces is fairly subtle.  I think we are able to decode other
subpicture bitstreams bitexact.  SUBPIC_A_HUAWEI_3 falls into this last
category.

I posted a patch, like yours Fei, which errors only the most problematic
pps_single_slice_per_subpic_flag bitstreams some time ago:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240201140055.63805-1-post@frankplowman.com/
and the feedback I received there was to make this patch.  I have also
implemented the logic needed for pps_single_slice_per_subpic_flag here:
https://github.com/ffvvc/FFmpeg/pull/191.  I would be happy to make a v2
of the pps_single_slice_per_subpic_flag patch which moves where the
error is being returned, or to put the GitHub PR on the ML if people
would rather one of those two alternatives.

Thanks again,
-- 
Frank