Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/2] lavc/vvc: Fix slice map construction for small subpics
@ 2025-02-09 15:43 Frank Plowman
  2025-02-09 15:43 ` [FFmpeg-devel] [PATCH 2/2] lavc/vvc: Ensure subpictures don't overlap Frank Plowman
  0 siblings, 1 reply; 2+ messages in thread
From: Frank Plowman @ 2025-02-09 15:43 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Frank Plowman, nuomi2021

In the case pps_subpic_less_than_one_tile_slice is called, the
subpicture is smaller than the tile and so there are multiple
subpictures in the tile.  Of course, then, not all the
subpictures can start in the top-left corner as the code before the
patch does.  Patch fixes this, so each subpicture starts at the
signalled location as is specified in section 6.5.1 of H.266(V3).

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

diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index fae6655cc0..9480540e03 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -404,8 +404,8 @@ static void subpic_tiles(int *tile_x, int *tile_y, int *tile_x_end, int *tile_y_
 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)
 {
     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], sps->r->sps_subpic_height_minus1[i] + 1);
+        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);
 }
 
 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)
-- 
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".

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] lavc/vvc: Ensure subpictures don't overlap
  2025-02-09 15:43 [FFmpeg-devel] [PATCH 1/2] lavc/vvc: Fix slice map construction for small subpics Frank Plowman
@ 2025-02-09 15:43 ` Frank Plowman
  0 siblings, 0 replies; 2+ messages in thread
From: Frank Plowman @ 2025-02-09 15:43 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Frank Plowman, nuomi2021

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".

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-02-09 15:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-09 15:43 [FFmpeg-devel] [PATCH 1/2] lavc/vvc: Fix slice map construction for small subpics Frank Plowman
2025-02-09 15:43 ` [FFmpeg-devel] [PATCH 2/2] lavc/vvc: Ensure subpictures don't overlap Frank Plowman

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git