Fuzz ID 283

[frankplow]

Fuzz bitstream ID 283 crashes because of a bad read here: https://github.com/ffvvc/FFmpeg/blob/8670615743eb36d7b3e9b522266518796df7ec54/libavcodec/vvc/mvs.c#L247 This happens because the collocated reference, which the MVF table is from, has a different (smaller) resolution to the current frame. This seems to be disallowed by the standard (VVCv3 p. 157):

Let colPicList be set equal to sh_collocated_from_l0_flag ? 0 : 1. It is a requirement of bitstream conformance that the picture referred to by sh_collocated_ref_idx shall be the same for all non-I slices of a coded picture, the value of RprConstraintsActiveFlag[ colPicList ][ sh_collocated_ref_idx ] shall be equal to 0, and the value of sps_log2_ctu_size_minus5 for the picture referred to by sh_collocated_ref_idx shall be equal to the value of sps_log2_ctu_size_minus5 for the current picture.

NOTE 2 – The collocated picture has the same spatial resolution, the same scaling window offsets, the same number of subpictures, and the same CTU size as the current picture.

The patch below is an attempt at implementing these requirements, excluding that sh_collocated_ref_idx must be the same for all non-I slices of the current frame.

diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index fb42963034..82445ca44b 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -507,8 +507,12 @@ int ff_vvc_slice_rpl(VVCContext *s, VVCFrameContext *fc, SliceContext *sc)
             }
         }
         if ((!rsh->sh_collocated_from_l0_flag) == lx &&
-            rsh->sh_collocated_ref_idx < rpl->nb_refs)
+            rsh->sh_collocated_ref_idx < rpl->nb_refs) {
+            VVCRefPic ref = rpl->refs[rsh->sh_collocated_ref_idx];
+            if (ref.is_scaled || ref.ref->sps->ctb_log2_size_y != sps->ctb_log2_size_y)
+                return AVERROR_INVALIDDATA;
             fc->ref->collocated_ref = rpl->refs[rsh->sh_collocated_ref_idx].ref;
+        }
     }
     return 0;
 }

The patch fixes the fuzz failure, but breaks RPR decoding. In general, I do not see how the collocated references work for the first picture after a resolution change with RPR. If there are no pictures in the RPL with the same resolution as the current one, will the requirements in the excerpt above not always be violated?

[nuomi2021]

@frankplow , maybe we can check the collocated frame at https://github.com/FFmpeg/FFmpeg/blob/62397bcf6a5b4052388ab4898a747cda56abc082/libavcodec/vvc/refs.c#L511 if it is a scaled ref, report AVERROR_INVALIDDATA

[frankplow]

@nuomi2021

@frankplow , maybe we can check the collocated frame at https://github.com/FFmpeg/FFmpeg/blob/62397bcf6a5b4052388ab4898a747cda56abc082/libavcodec/vvc/refs.c#L511 if it is a scaled ref, report AVERROR_INVALIDDATA

That is what the patch I posted above does.

[nuomi2021]

Oh, sorry, I missed this code during my quick scan. We need to check ph_temporal_mvp_enabled_flag too.

diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index fb42963034..40b7a7b919 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -506,9 +506,16 @@ int ff_vvc_slice_rpl(VVCContext *s, VVCFrameContext *fc, SliceContext *sc)
                 return ret;
             }
         }
-        if ((!rsh->sh_collocated_from_l0_flag) == lx &&
-            rsh->sh_collocated_ref_idx < rpl->nb_refs)
-            fc->ref->collocated_ref = rpl->refs[rsh->sh_collocated_ref_idx].ref;
+        if (ph->r->ph_temporal_mvp_enabled_flag &&
+            ((!rsh->sh_collocated_from_l0_flag) == lx &&
+                rsh->sh_collocated_ref_idx < rpl->nb_refs)) {
+            const VVCRefPic *refp = rpl->refs + rsh->sh_collocated_ref_idx;
+            VVCFrame *ref             = refp->ref;
+
+            if (refp->is_scaled || ref->sps->ctb_log2_size_y != sps->ctb_log2_size_y)
+                return AVERROR_INVALIDDATA;
+            fc->ref->collocated_ref = ref;
+        }
     }
     return 0;
 }

By the way, ph_temporal_mvp_enabled_flag is checked by both temporal_luma_motion_vector and its caller. If you're interested, it would be a good practice to remove the duplicate check.

[nuomi2021]

@frankplow 👍, this will fix about 4 fuzz clips on my local

[frankplow]

Patch sent to upstream ML: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240526091618.24432-1-post@frankplowman.com/

[frankplow]

Fixed in https://github.com/FFmpeg/FFmpeg/commit/49c3918c1ac64b4ae3ab61b9862ec75c96b2b6c2