[FFmpeg-devel] [PATCH 2/2] lavc/vvc: Ensure subpictures don't overlap

[Frank Plowman <post@frankplowman.com>]

This is essentially a re-implementation of
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20241005223955.54158-1-post@frankplowman.com/

That patch was not applied last time.  Instead we opted to identify
issues which could be caused by invalid subpicture layouts and remedy
those issues where they manifest, either through error detection or code
hardening.  This was primarily implemented in the set
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=13381.

This has worked to some degree, however issues with subpicture layouts
continue to crop up from the fuzzer and I've fixed a number of bugs
related to subpicture layouts since then.  I think it's best to return
to the initial plan and simply check if the subpicture layout is valid
initially.

This implementation is also lighter than the first time -- by doing a
bit more logic in pps_subpic_less_than_one_tile_slice, we are able to
store a tile_in_subpic map rather than a ctu_in_subpic map.  This
reduces the size of the map to the point it becomes possible to allocate
it on the stack.  Similar to 8bd66a8c9587af61c7b46558be3c4ee317c1af5a,
the layout is also validated in the slice map construction code, rather
than in the CBS, which avoids duplicating some logic.

Signed-off-by: Frank Plowman <post@frankplowman.com>
---
 libavcodec/vvc/ps.c | 52 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index 9480540e03..9af5e1250b 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -401,25 +401,47 @@ static void subpic_tiles(int *tile_x, int *tile_y, int *tile_x_end, int *tile_y_
         (*tile_y_end)++;
 }
 
-static void pps_subpic_less_than_one_tile_slice(VVCPPS *pps, const VVCSPS *sps, const int i, const int tx, const int ty, int *off)
+static int pps_subpic_less_than_one_tile_slice(VVCPPS *pps, const VVCSPS *sps, const int i, const int tx, const int ty, int *off, bool *tile_in_subpic)
 {
+    const int subpic_right = sps->r->sps_subpic_ctu_top_left_x[i] + sps->r->sps_subpic_width_minus1[i];
+    const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] + sps->r->sps_subpic_height_minus1[i];
+    const int tile_right = pps->col_bd[tx] + pps->r->col_width_val[tx] - 1;
+    const int tile_bottom = pps->row_bd[ty] + pps->r->row_height_val[ty] - 1;
+    const bool is_final_subpic_in_tile = subpic_right == tile_right && subpic_bottom == tile_bottom;
+
+    if (is_final_subpic_in_tile) {
+        const size_t tile_idx = ty * VVC_MAX_TILE_COLUMNS + tx;
+        if (tile_in_subpic[tile_idx])
+            return AVERROR_INVALIDDATA;
+        tile_in_subpic[tile_idx] = true;
+    }
+
     pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off,
         sps->r->sps_subpic_ctu_top_left_x[i], sps->r->sps_subpic_ctu_top_left_y[i],
         sps->r->sps_subpic_width_minus1[i] + 1, sps->r->sps_subpic_height_minus1[i] + 1);
+
+    return 0;
 }
 
-static void pps_subpic_one_or_more_tiles_slice(VVCPPS *pps, const int tile_x, const int tile_y, const int x_end, const int y_end, const int i, int *off)
+static int pps_subpic_one_or_more_tiles_slice(VVCPPS *pps, const int tile_x, const int tile_y, const int x_end, const int y_end,
+    const int i, int *off, bool *tile_in_subpic)
 {
     for (int ty = tile_y; ty < y_end; ty++) {
         for (int tx = tile_x; tx < x_end; tx++) {
+            const size_t tile_idx = ty * VVC_MAX_TILE_COLUMNS + tx;
+            if (tile_in_subpic[tile_idx])
+                return AVERROR_INVALIDDATA;
+            tile_in_subpic[tile_idx] = true;
+
             pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off,
                 pps->col_bd[tx], pps->row_bd[ty],
                 pps->r->col_width_val[tx], pps->r->row_height_val[ty]);
         }
     }
+    return 0;
 }
 
-static void pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *off)
+static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *off, bool *tile_in_subpic)
 {
     int tx, ty, x_end, y_end;
 
@@ -428,19 +450,30 @@ static void pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *o
 
     subpic_tiles(&tx, &ty, &x_end, &y_end, sps, pps, i);
     if (ty + 1 == y_end && sps->r->sps_subpic_height_minus1[i] + 1 < pps->r->row_height_val[ty])
-        pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty, off);
+        return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty, off, tile_in_subpic);
     else
-        pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end, y_end, i, off);
+        return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end, y_end, i, off, tile_in_subpic);
 }
 
-static void pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps, int *off)
+static int pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps, int *off)
 {
     if (!sps->r->sps_subpic_info_present_flag) {
         pps_single_slice_picture(pps, off);
     } else {
-        for (int i = 0; i < pps->r->pps_num_slices_in_pic_minus1 + 1; i++)
-            pps_subpic_slice(pps, sps, i, off);
+        bool tile_in_subpic[VVC_MAX_TILES_PER_AU] = {0};
+        for (int i = 0; i < pps->r->pps_num_slices_in_pic_minus1 + 1; i++) {
+            const int ret = pps_subpic_slice(pps, sps, i, off, tile_in_subpic);
+            if (ret < 0)
+                return ret;
+        }
+
+        // We only use tile_in_subpic to check that the subpictures don't overlap
+        // here; we don't use tile_in_subpic to check that the subpictures cover
+        // every tile.  It is possible to avoid doing this work here because the
+        // covering property of subpictures is already guaranteed by the mechanisms
+        // which check every CTU belongs to a slice.
     }
+    return 0;
 }
 
 static int pps_one_tile_slices(VVCPPS *pps, const int tile_idx, int i, int *off)
@@ -491,8 +524,7 @@ static int pps_rect_slice(VVCPPS *pps, const VVCSPS *sps)
     int tile_idx = 0, off = 0;
 
     if (r->pps_single_slice_per_subpic_flag) {
-        pps_single_slice_per_subpic(pps, sps, &off);
-        return 0;
+        return pps_single_slice_per_subpic(pps, sps, &off);
     }
 
     for (int i = 0; i < r->pps_num_slices_in_pic_minus1 + 1; i++) {
-- 
2.47.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".