* [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level
@ 2025-04-27 8:47 Frank Plowman
2025-04-28 13:33 ` Nuo Mi
0 siblings, 1 reply; 6+ messages in thread
From: Frank Plowman @ 2025-04-27 8:47 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Frank Plowman, nuomi2021
In d5dbcc00d889fb17948b025a468b00ddbea9e058, it was hoped that detection
of subpicture overlaps could be performed at the tile level, so as to
avoid introducing per-CTU checks. Unfortunately since that patch,
fuzzing has indicated there are some structures involving
pps_subpic_one_or_more_tiles_slice where tile-level checking is not
sufficient. Performing the check at the CTU level should (touch wood)
be the be-all and and-all of this, as CTUs are the lowest common
denominator of the picture partitioning.
Signed-off-by: Frank Plowman <post@frankplowman.com>
---
Changes since v1:
* Merge pps_add_ctus and pps_add_ctus_check
* Change if/else for early-exit where possible
---
libavcodec/vvc/ps.c | 71 ++++++++++++++++++++-------------------------
1 file changed, 31 insertions(+), 40 deletions(-)
diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index e8c312d8ac..ed96268bae 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -408,6 +408,8 @@ static int pps_add_ctus(VVCPPS *pps, int *off, const int rx, const int ry,
int start = *off;
for (int y = 0; y < h; y++) {
for (int x = 0; x < w; x++) {
+ if (*off >= pps->ctb_count)
+ return AVERROR_INVALIDDATA;
pps->ctb_addr_in_slice[*off] = ctu_rs(rx + x, ry + y, pps);
(*off)++;
}
@@ -420,9 +422,11 @@ static void pps_single_slice_picture(VVCPPS *pps, int *off)
pps->num_ctus_in_slice[0] = 0;
for (int j = 0; j < pps->r->num_tile_rows; j++) {
for (int i = 0; i < pps->r->num_tile_columns; i++) {
- pps->num_ctus_in_slice[0] += pps_add_ctus(pps, off,
+ const int ret = pps_add_ctus(pps, off,
pps->col_bd[i], pps->row_bd[j],
pps->r->col_width_val[i], pps->r->row_height_val[j]);
+ av_assert2(ret >= 0);
+ pps->num_ctus_in_slice[0] += ret;
}
}
}
@@ -451,50 +455,36 @@ static void subpic_tiles(int *tile_x, int *tile_y, int *tile_x_end, int *tile_y_
(*tile_y_end)++;
}
-static bool mark_tile_as_used(bool *tile_in_subpic, const int tx, const int ty, const int tile_columns)
-{
- const size_t tile_idx = ty * tile_columns + tx;
- if (tile_in_subpic[tile_idx]) {
- /* the tile is covered by other subpictures */
- return false;
- }
- tile_in_subpic[tile_idx] = true;
- return true;
-}
-
-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)
+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)
{
- const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] + sps->r->sps_subpic_height_minus1[i];
- const int tile_bottom = pps->row_bd[ty] + pps->r->row_height_val[ty] - 1;
- const bool is_final_subpic_in_tile = subpic_bottom == tile_bottom;
-
- if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic, tx, ty, pps->r->num_tile_columns))
- return AVERROR_INVALIDDATA;
-
- pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off,
+ const int ret = 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);
+ if (ret < 0)
+ return ret;
+ pps->num_ctus_in_slice[i] = ret;
return 0;
}
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)
+ const int i, int *off)
{
for (int ty = tile_y; ty < y_end; ty++) {
for (int tx = tile_x; tx < x_end; tx++) {
- if (!mark_tile_as_used(tile_in_subpic, tx, ty, pps->r->num_tile_columns))
- return AVERROR_INVALIDDATA;
-
- pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off,
+ const int ret = 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]);
+ if (ret < 0)
+ return ret;
+
+ pps->num_ctus_in_slice[i] += ret;
}
}
return 0;
}
-static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *off, bool *tile_in_subpic)
+static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *off)
{
int tx, ty, x_end, y_end;
@@ -503,9 +493,9 @@ static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *of
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])
- return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty, off, tile_in_subpic);
+ return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty, off);
else
- return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end, y_end, i, off, tile_in_subpic);
+ return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end, y_end, i, off);
}
static int pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps, int *off)
@@ -513,18 +503,11 @@ 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 {
- 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);
+ const int ret = pps_subpic_slice(pps, sps, i, off);
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;
}
@@ -538,9 +521,12 @@ static int pps_one_tile_slices(VVCPPS *pps, const int tile_idx, int i, int *off)
ctu_xy(&rx, &ry, tile_x, tile_y, pps);
ctu_y_end = ry + r->row_height_val[tile_y];
while (ry < ctu_y_end) {
+ int ret;
pps->slice_start_offset[i] = *off;
- pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, rx, ry,
+ ret = pps_add_ctus(pps, off, rx, ry,
r->col_width_val[tile_x], r->slice_height_in_ctus[i]);
+ av_assert2(ret >= 0);
+ pps->num_ctus_in_slice[i] = ret;
ry += r->slice_height_in_ctus[i++];
}
i--;
@@ -557,13 +543,16 @@ static int pps_multi_tiles_slice(VVCPPS *pps, const int tile_idx, const int i, i
pps->num_ctus_in_slice[i] = 0;
for (int ty = tile_y; ty <= tile_y + r->pps_slice_height_in_tiles_minus1[i]; ty++) {
for (int tx = tile_x; tx <= tile_x + r->pps_slice_width_in_tiles_minus1[i]; tx++) {
+ int ret;
const int idx = ty * r->num_tile_columns + tx;
if (tile_in_slice[idx])
return AVERROR_INVALIDDATA;
tile_in_slice[idx] = true;
ctu_xy(&rx, &ry, tx, ty, pps);
- pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off, rx, ry,
+ ret = pps_add_ctus(pps, off, rx, ry,
r->col_width_val[tx], r->row_height_val[ty]);
+ av_assert2(ret >= 0);
+ pps->num_ctus_in_slice[i] += ret;
}
}
@@ -610,8 +599,10 @@ static void pps_no_rect_slice(VVCPPS* pps)
for (int tile_y = 0; tile_y < r->num_tile_rows; tile_y++) {
for (int tile_x = 0; tile_x < r->num_tile_columns; tile_x++) {
+ int ret;
ctu_xy(&rx, &ry, tile_x, tile_y, pps);
- pps_add_ctus(pps, &off, rx, ry, r->col_width_val[tile_x], r->row_height_val[tile_y]);
+ ret = pps_add_ctus(pps, &off, rx, ry, r->col_width_val[tile_x], r->row_height_val[tile_y]);
+ av_assert2(ret >= 0);
}
}
}
--
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] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level
2025-04-27 8:47 [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level Frank Plowman
@ 2025-04-28 13:33 ` Nuo Mi
2025-04-28 14:34 ` Frank Plowman
0 siblings, 1 reply; 6+ messages in thread
From: Nuo Mi @ 2025-04-28 13:33 UTC (permalink / raw)
To: Frank Plowman; +Cc: ffmpeg-devel
Hi Frank,
Thank you for the v2.
Could we remove all asserts?
Asserts can cause the application to crash at runtime.
On Sun, Apr 27, 2025 at 4:48 PM Frank Plowman <post@frankplowman.com> wrote:
> In d5dbcc00d889fb17948b025a468b00ddbea9e058, it was hoped that detection
> of subpicture overlaps could be performed at the tile level, so as to
> avoid introducing per-CTU checks. Unfortunately since that patch,
> fuzzing has indicated there are some structures involving
> pps_subpic_one_or_more_tiles_slice where tile-level checking is not
> sufficient. Performing the check at the CTU level should (touch wood)
> be the be-all and and-all of this, as CTUs are the lowest common
> denominator of the picture partitioning.
>
> Signed-off-by: Frank Plowman <post@frankplowman.com>
> ---
> Changes since v1:
> * Merge pps_add_ctus and pps_add_ctus_check
> * Change if/else for early-exit where possible
>
> ---
> libavcodec/vvc/ps.c | 71 ++++++++++++++++++++-------------------------
> 1 file changed, 31 insertions(+), 40 deletions(-)
>
> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
> index e8c312d8ac..ed96268bae 100644
> --- a/libavcodec/vvc/ps.c
> +++ b/libavcodec/vvc/ps.c
> @@ -408,6 +408,8 @@ static int pps_add_ctus(VVCPPS *pps, int *off, const
> int rx, const int ry,
> int start = *off;
> for (int y = 0; y < h; y++) {
> for (int x = 0; x < w; x++) {
> + if (*off >= pps->ctb_count)
> + return AVERROR_INVALIDDATA;
> pps->ctb_addr_in_slice[*off] = ctu_rs(rx + x, ry + y, pps);
> (*off)++;
> }
> @@ -420,9 +422,11 @@ static void pps_single_slice_picture(VVCPPS *pps, int
> *off)
> pps->num_ctus_in_slice[0] = 0;
> for (int j = 0; j < pps->r->num_tile_rows; j++) {
> for (int i = 0; i < pps->r->num_tile_columns; i++) {
> - pps->num_ctus_in_slice[0] += pps_add_ctus(pps, off,
> + const int ret = pps_add_ctus(pps, off,
> pps->col_bd[i], pps->row_bd[j],
> pps->r->col_width_val[i], pps->r->row_height_val[j]);
> + av_assert2(ret >= 0);
> + pps->num_ctus_in_slice[0] += ret;
> }
> }
> }
> @@ -451,50 +455,36 @@ static void subpic_tiles(int *tile_x, int *tile_y,
> int *tile_x_end, int *tile_y_
> (*tile_y_end)++;
> }
>
> -static bool mark_tile_as_used(bool *tile_in_subpic, const int tx, const
> int ty, const int tile_columns)
> -{
> - const size_t tile_idx = ty * tile_columns + tx;
> - if (tile_in_subpic[tile_idx]) {
> - /* the tile is covered by other subpictures */
> - return false;
> - }
> - tile_in_subpic[tile_idx] = true;
> - return true;
> -}
> -
> -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)
> +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)
> {
> - const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] +
> sps->r->sps_subpic_height_minus1[i];
> - const int tile_bottom = pps->row_bd[ty] + pps->r->row_height_val[ty]
> - 1;
> - const bool is_final_subpic_in_tile = subpic_bottom == tile_bottom;
> -
> - if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic, tx,
> ty, pps->r->num_tile_columns))
> - return AVERROR_INVALIDDATA;
> -
> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off,
> + const int ret = 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);
> + if (ret < 0)
> + return ret;
>
> + pps->num_ctus_in_slice[i] = ret;
> return 0;
> }
>
> 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)
> + const int i, int *off)
> {
> for (int ty = tile_y; ty < y_end; ty++) {
> for (int tx = tile_x; tx < x_end; tx++) {
> - if (!mark_tile_as_used(tile_in_subpic, tx, ty,
> pps->r->num_tile_columns))
> - return AVERROR_INVALIDDATA;
> -
> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off,
> + const int ret = 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]);
> + if (ret < 0)
> + return ret;
> +
> + pps->num_ctus_in_slice[i] += ret;
> }
> }
> return 0;
> }
>
> -static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i,
> int *off, bool *tile_in_subpic)
> +static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i,
> int *off)
> {
> int tx, ty, x_end, y_end;
>
> @@ -503,9 +493,9 @@ static int pps_subpic_slice(VVCPPS *pps, const VVCSPS
> *sps, const int i, int *of
>
> 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])
> - return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty,
> off, tile_in_subpic);
> + return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty,
> off);
> else
> - return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
> y_end, i, off, tile_in_subpic);
> + return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
> y_end, i, off);
> }
>
> static int pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps,
> int *off)
> @@ -513,18 +503,11 @@ 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 {
> - 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);
> + const int ret = pps_subpic_slice(pps, sps, i, off);
> 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;
> }
> @@ -538,9 +521,12 @@ static int pps_one_tile_slices(VVCPPS *pps, const int
> tile_idx, int i, int *off)
> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
> ctu_y_end = ry + r->row_height_val[tile_y];
> while (ry < ctu_y_end) {
> + int ret;
> pps->slice_start_offset[i] = *off;
> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, rx, ry,
> + ret = pps_add_ctus(pps, off, rx, ry,
> r->col_width_val[tile_x], r->slice_height_in_ctus[i]);
> + av_assert2(ret >= 0);
> + pps->num_ctus_in_slice[i] = ret;
> ry += r->slice_height_in_ctus[i++];
> }
> i--;
> @@ -557,13 +543,16 @@ static int pps_multi_tiles_slice(VVCPPS *pps, const
> int tile_idx, const int i, i
> pps->num_ctus_in_slice[i] = 0;
> for (int ty = tile_y; ty <= tile_y +
> r->pps_slice_height_in_tiles_minus1[i]; ty++) {
> for (int tx = tile_x; tx <= tile_x +
> r->pps_slice_width_in_tiles_minus1[i]; tx++) {
> + int ret;
> const int idx = ty * r->num_tile_columns + tx;
> if (tile_in_slice[idx])
> return AVERROR_INVALIDDATA;
> tile_in_slice[idx] = true;
> ctu_xy(&rx, &ry, tx, ty, pps);
> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off, rx, ry,
> + ret = pps_add_ctus(pps, off, rx, ry,
> r->col_width_val[tx], r->row_height_val[ty]);
> + av_assert2(ret >= 0);
> + pps->num_ctus_in_slice[i] += ret;
> }
> }
>
> @@ -610,8 +599,10 @@ static void pps_no_rect_slice(VVCPPS* pps)
>
> for (int tile_y = 0; tile_y < r->num_tile_rows; tile_y++) {
> for (int tile_x = 0; tile_x < r->num_tile_columns; tile_x++) {
> + int ret;
> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
> - pps_add_ctus(pps, &off, rx, ry, r->col_width_val[tile_x],
> r->row_height_val[tile_y]);
> + ret = pps_add_ctus(pps, &off, rx, ry,
> r->col_width_val[tile_x], r->row_height_val[tile_y]);
> + av_assert2(ret >= 0);
> }
> }
> }
> --
> 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] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level
2025-04-28 13:33 ` Nuo Mi
@ 2025-04-28 14:34 ` Frank Plowman
2025-04-29 13:24 ` Nuo Mi
0 siblings, 1 reply; 6+ messages in thread
From: Frank Plowman @ 2025-04-28 14:34 UTC (permalink / raw)
To: ffmpeg-devel
On 28/04/2025 14:33, Nuo Mi wrote:
> Hi Frank,
> Thank you for the v2.
> Could we remove all asserts?
> Asserts can cause the application to crash at runtime.
Hi,
I think av_assert2s are the right thing to use here. In case it was not
clear, these asserts should never be triggered by any bitstream (legal
or illegal). The alternatives, as I see it, are both less favourable:
* Don't check the return value at all. If the assumption above that
pps_add_ctus shouldn't fail in these cases is incorrect, then all of a
sudden there is a rather unscrutable error arising from subtracting a
value from off, which might be rather difficult to debug. An assertion
is better because it makes the issue obvious by crashing, and
immediately points to the location in the code which is problematic.
* Add a runtime check for these cases. If the assumption above is
correct, then we're incurring needless runtime penalty checking for
things which are always true. An av_assert2 is better because it is
only enabled in debug builds, and not where performance is essential.
>
> On Sun, Apr 27, 2025 at 4:48 PM Frank Plowman <post@frankplowman.com> wrote:
>
>> In d5dbcc00d889fb17948b025a468b00ddbea9e058, it was hoped that detection
>> of subpicture overlaps could be performed at the tile level, so as to
>> avoid introducing per-CTU checks. Unfortunately since that patch,
>> fuzzing has indicated there are some structures involving
>> pps_subpic_one_or_more_tiles_slice where tile-level checking is not
>> sufficient. Performing the check at the CTU level should (touch wood)
>> be the be-all and and-all of this, as CTUs are the lowest common
>> denominator of the picture partitioning.
>>
>> Signed-off-by: Frank Plowman <post@frankplowman.com>
>> ---
>> Changes since v1:
>> * Merge pps_add_ctus and pps_add_ctus_check
>> * Change if/else for early-exit where possible
>>
>> ---
>> libavcodec/vvc/ps.c | 71 ++++++++++++++++++++-------------------------
>> 1 file changed, 31 insertions(+), 40 deletions(-)
>>
>> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
>> index e8c312d8ac..ed96268bae 100644
>> --- a/libavcodec/vvc/ps.c
>> +++ b/libavcodec/vvc/ps.c
>> @@ -408,6 +408,8 @@ static int pps_add_ctus(VVCPPS *pps, int *off, const
>> int rx, const int ry,
>> int start = *off;
>> for (int y = 0; y < h; y++) {
>> for (int x = 0; x < w; x++) {
>> + if (*off >= pps->ctb_count)
>> + return AVERROR_INVALIDDATA;
>> pps->ctb_addr_in_slice[*off] = ctu_rs(rx + x, ry + y, pps);
>> (*off)++;
>> }
>> @@ -420,9 +422,11 @@ static void pps_single_slice_picture(VVCPPS *pps, int
>> *off)
>> pps->num_ctus_in_slice[0] = 0;
>> for (int j = 0; j < pps->r->num_tile_rows; j++) {
>> for (int i = 0; i < pps->r->num_tile_columns; i++) {
>> - pps->num_ctus_in_slice[0] += pps_add_ctus(pps, off,
>> + const int ret = pps_add_ctus(pps, off,
>> pps->col_bd[i], pps->row_bd[j],
>> pps->r->col_width_val[i], pps->r->row_height_val[j]);
>> + av_assert2(ret >= 0);
>> + pps->num_ctus_in_slice[0] += ret;
>> }
>> }
>> }
>> @@ -451,50 +455,36 @@ static void subpic_tiles(int *tile_x, int *tile_y,
>> int *tile_x_end, int *tile_y_
>> (*tile_y_end)++;
>> }
>>
>> -static bool mark_tile_as_used(bool *tile_in_subpic, const int tx, const
>> int ty, const int tile_columns)
>> -{
>> - const size_t tile_idx = ty * tile_columns + tx;
>> - if (tile_in_subpic[tile_idx]) {
>> - /* the tile is covered by other subpictures */
>> - return false;
>> - }
>> - tile_in_subpic[tile_idx] = true;
>> - return true;
>> -}
>> -
>> -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)
>> +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)
>> {
>> - const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] +
>> sps->r->sps_subpic_height_minus1[i];
>> - const int tile_bottom = pps->row_bd[ty] + pps->r->row_height_val[ty]
>> - 1;
>> - const bool is_final_subpic_in_tile = subpic_bottom == tile_bottom;
>> -
>> - if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic, tx,
>> ty, pps->r->num_tile_columns))
>> - return AVERROR_INVALIDDATA;
>> -
>> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off,
>> + const int ret = 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);
>> + if (ret < 0)
>> + return ret;
>>
>> + pps->num_ctus_in_slice[i] = ret;
>> return 0;
>> }
>>
>> 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)
>> + const int i, int *off)
>> {
>> for (int ty = tile_y; ty < y_end; ty++) {
>> for (int tx = tile_x; tx < x_end; tx++) {
>> - if (!mark_tile_as_used(tile_in_subpic, tx, ty,
>> pps->r->num_tile_columns))
>> - return AVERROR_INVALIDDATA;
>> -
>> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off,
>> + const int ret = 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]);
>> + if (ret < 0)
>> + return ret;
>> +
>> + pps->num_ctus_in_slice[i] += ret;
>> }
>> }
>> return 0;
>> }
>>
>> -static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i,
>> int *off, bool *tile_in_subpic)
>> +static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i,
>> int *off)
>> {
>> int tx, ty, x_end, y_end;
>>
>> @@ -503,9 +493,9 @@ static int pps_subpic_slice(VVCPPS *pps, const VVCSPS
>> *sps, const int i, int *of
>>
>> 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])
>> - return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty,
>> off, tile_in_subpic);
>> + return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty,
>> off);
>> else
>> - return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
>> y_end, i, off, tile_in_subpic);
>> + return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
>> y_end, i, off);
>> }
>>
>> static int pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps,
>> int *off)
>> @@ -513,18 +503,11 @@ 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 {
>> - 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);
>> + const int ret = pps_subpic_slice(pps, sps, i, off);
>> 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;
>> }
>> @@ -538,9 +521,12 @@ static int pps_one_tile_slices(VVCPPS *pps, const int
>> tile_idx, int i, int *off)
>> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
>> ctu_y_end = ry + r->row_height_val[tile_y];
>> while (ry < ctu_y_end) {
>> + int ret;
>> pps->slice_start_offset[i] = *off;
>> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, rx, ry,
>> + ret = pps_add_ctus(pps, off, rx, ry,
>> r->col_width_val[tile_x], r->slice_height_in_ctus[i]);
>> + av_assert2(ret >= 0);
>> + pps->num_ctus_in_slice[i] = ret;
>> ry += r->slice_height_in_ctus[i++];
>> }
>> i--;
>> @@ -557,13 +543,16 @@ static int pps_multi_tiles_slice(VVCPPS *pps, const
>> int tile_idx, const int i, i
>> pps->num_ctus_in_slice[i] = 0;
>> for (int ty = tile_y; ty <= tile_y +
>> r->pps_slice_height_in_tiles_minus1[i]; ty++) {
>> for (int tx = tile_x; tx <= tile_x +
>> r->pps_slice_width_in_tiles_minus1[i]; tx++) {
>> + int ret;
>> const int idx = ty * r->num_tile_columns + tx;
>> if (tile_in_slice[idx])
>> return AVERROR_INVALIDDATA;
>> tile_in_slice[idx] = true;
>> ctu_xy(&rx, &ry, tx, ty, pps);
>> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off, rx, ry,
>> + ret = pps_add_ctus(pps, off, rx, ry,
>> r->col_width_val[tx], r->row_height_val[ty]);
>> + av_assert2(ret >= 0);
>> + pps->num_ctus_in_slice[i] += ret;
>> }
>> }
>>
>> @@ -610,8 +599,10 @@ static void pps_no_rect_slice(VVCPPS* pps)
>>
>> for (int tile_y = 0; tile_y < r->num_tile_rows; tile_y++) {
>> for (int tile_x = 0; tile_x < r->num_tile_columns; tile_x++) {
>> + int ret;
>> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
>> - pps_add_ctus(pps, &off, rx, ry, r->col_width_val[tile_x],
>> r->row_height_val[tile_y]);
>> + ret = pps_add_ctus(pps, &off, rx, ry,
>> r->col_width_val[tile_x], r->row_height_val[tile_y]);
>> + av_assert2(ret >= 0);
>> }
>> }
>> }
>> --
>> 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".
_______________________________________________
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] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level
2025-04-28 14:34 ` Frank Plowman
@ 2025-04-29 13:24 ` Nuo Mi
2025-05-18 13:16 ` Frank Plowman
0 siblings, 1 reply; 6+ messages in thread
From: Nuo Mi @ 2025-04-29 13:24 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Hi Frank,
Thank you for the detail.
On Mon, Apr 28, 2025 at 10:35 PM Frank Plowman <post@frankplowman.com>
wrote:
> On 28/04/2025 14:33, Nuo Mi wrote:
> > Hi Frank,
> > Thank you for the v2.
> > Could we remove all asserts?
> > Asserts can cause the application to crash at runtime.
>
> Hi,
>
> I think av_assert2s are the right thing to use here. In case it was not
> clear, these asserts should never be triggered by any bitstream (legal
> or illegal). The alternatives, as I see it, are both less favourable:
>
> * Don't check the return value at all. If the assumption above that
> pps_add_ctus shouldn't fail in these cases is incorrect, then all of a
> sudden there is a rather unscrutable error arising from subtracting a
> value from off, which might be rather difficult to debug. An assertion
> is better because it makes the issue obvious by crashing, and
> immediately points to the location in the code which is problematic.
>
FFmpeg will run on multiple computers, so detecting and returning an error
is better than crashing the program — even if it's a rare occurrence.
>
> * Add a runtime check for these cases. If the assumption above is
> correct, then we're incurring needless runtime penalty checking for
> things which are always true. An av_assert2 is better because it is
> only enabled in debug builds, and not where performance is essential.
>
The entire process happens at the PPS level, and usually, we only have one
per stream, so the performance loss should be minimal.
>
> >
> > On Sun, Apr 27, 2025 at 4:48 PM Frank Plowman <post@frankplowman.com>
> wrote:
> >
> >> In d5dbcc00d889fb17948b025a468b00ddbea9e058, it was hoped that detection
> >> of subpicture overlaps could be performed at the tile level, so as to
> >> avoid introducing per-CTU checks. Unfortunately since that patch,
> >> fuzzing has indicated there are some structures involving
> >> pps_subpic_one_or_more_tiles_slice where tile-level checking is not
> >> sufficient. Performing the check at the CTU level should (touch wood)
> >> be the be-all and and-all of this, as CTUs are the lowest common
> >> denominator of the picture partitioning.
> >>
> >> Signed-off-by: Frank Plowman <post@frankplowman.com>
> >> ---
> >> Changes since v1:
> >> * Merge pps_add_ctus and pps_add_ctus_check
> >> * Change if/else for early-exit where possible
> >>
> >> ---
> >> libavcodec/vvc/ps.c | 71 ++++++++++++++++++++-------------------------
> >> 1 file changed, 31 insertions(+), 40 deletions(-)
> >>
> >> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
> >> index e8c312d8ac..ed96268bae 100644
> >> --- a/libavcodec/vvc/ps.c
> >> +++ b/libavcodec/vvc/ps.c
> >> @@ -408,6 +408,8 @@ static int pps_add_ctus(VVCPPS *pps, int *off, const
> >> int rx, const int ry,
> >> int start = *off;
> >> for (int y = 0; y < h; y++) {
> >> for (int x = 0; x < w; x++) {
> >> + if (*off >= pps->ctb_count)
> >> + return AVERROR_INVALIDDATA;
> >> pps->ctb_addr_in_slice[*off] = ctu_rs(rx + x, ry + y, pps);
> >> (*off)++;
> >> }
> >> @@ -420,9 +422,11 @@ static void pps_single_slice_picture(VVCPPS *pps,
> int
> >> *off)
> >> pps->num_ctus_in_slice[0] = 0;
> >> for (int j = 0; j < pps->r->num_tile_rows; j++) {
> >> for (int i = 0; i < pps->r->num_tile_columns; i++) {
> >> - pps->num_ctus_in_slice[0] += pps_add_ctus(pps, off,
> >> + const int ret = pps_add_ctus(pps, off,
> >> pps->col_bd[i], pps->row_bd[j],
> >> pps->r->col_width_val[i], pps->r->row_height_val[j]);
> >> + av_assert2(ret >= 0);
> >> + pps->num_ctus_in_slice[0] += ret;
> >> }
> >> }
> >> }
> >> @@ -451,50 +455,36 @@ static void subpic_tiles(int *tile_x, int *tile_y,
> >> int *tile_x_end, int *tile_y_
> >> (*tile_y_end)++;
> >> }
> >>
> >> -static bool mark_tile_as_used(bool *tile_in_subpic, const int tx, const
> >> int ty, const int tile_columns)
> >> -{
> >> - const size_t tile_idx = ty * tile_columns + tx;
> >> - if (tile_in_subpic[tile_idx]) {
> >> - /* the tile is covered by other subpictures */
> >> - return false;
> >> - }
> >> - tile_in_subpic[tile_idx] = true;
> >> - return true;
> >> -}
> >> -
> >> -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)
> >> +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)
> >> {
> >> - const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] +
> >> sps->r->sps_subpic_height_minus1[i];
> >> - const int tile_bottom = pps->row_bd[ty] +
> pps->r->row_height_val[ty]
> >> - 1;
> >> - const bool is_final_subpic_in_tile = subpic_bottom == tile_bottom;
> >> -
> >> - if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic,
> tx,
> >> ty, pps->r->num_tile_columns))
> >> - return AVERROR_INVALIDDATA;
> >> -
> >> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off,
> >> + const int ret = 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);
> >> + if (ret < 0)
> >> + return ret;
> >>
> >> + pps->num_ctus_in_slice[i] = ret;
> >> return 0;
> >> }
> >>
> >> 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)
> >> + const int i, int *off)
> >> {
> >> for (int ty = tile_y; ty < y_end; ty++) {
> >> for (int tx = tile_x; tx < x_end; tx++) {
> >> - if (!mark_tile_as_used(tile_in_subpic, tx, ty,
> >> pps->r->num_tile_columns))
> >> - return AVERROR_INVALIDDATA;
> >> -
> >> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off,
> >> + const int ret = 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]);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + pps->num_ctus_in_slice[i] += ret;
> >> }
> >> }
> >> return 0;
> >> }
> >>
> >> -static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int
> i,
> >> int *off, bool *tile_in_subpic)
> >> +static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int
> i,
> >> int *off)
> >> {
> >> int tx, ty, x_end, y_end;
> >>
> >> @@ -503,9 +493,9 @@ static int pps_subpic_slice(VVCPPS *pps, const
> VVCSPS
> >> *sps, const int i, int *of
> >>
> >> 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])
> >> - return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty,
> >> off, tile_in_subpic);
> >> + return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty,
> >> off);
> >> else
> >> - return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
> >> y_end, i, off, tile_in_subpic);
> >> + return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
> >> y_end, i, off);
> >> }
> >>
> >> static int pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps,
> >> int *off)
> >> @@ -513,18 +503,11 @@ 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 {
> >> - 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);
> >> + const int ret = pps_subpic_slice(pps, sps, i, off);
> >> 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;
> >> }
> >> @@ -538,9 +521,12 @@ static int pps_one_tile_slices(VVCPPS *pps, const
> int
> >> tile_idx, int i, int *off)
> >> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
> >> ctu_y_end = ry + r->row_height_val[tile_y];
> >> while (ry < ctu_y_end) {
> >> + int ret;
> >> pps->slice_start_offset[i] = *off;
> >> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, rx, ry,
> >> + ret = pps_add_ctus(pps, off, rx, ry,
> >> r->col_width_val[tile_x], r->slice_height_in_ctus[i]);
> >> + av_assert2(ret >= 0);
> >> + pps->num_ctus_in_slice[i] = ret;
> >> ry += r->slice_height_in_ctus[i++];
> >> }
> >> i--;
> >> @@ -557,13 +543,16 @@ static int pps_multi_tiles_slice(VVCPPS *pps,
> const
> >> int tile_idx, const int i, i
> >> pps->num_ctus_in_slice[i] = 0;
> >> for (int ty = tile_y; ty <= tile_y +
> >> r->pps_slice_height_in_tiles_minus1[i]; ty++) {
> >> for (int tx = tile_x; tx <= tile_x +
> >> r->pps_slice_width_in_tiles_minus1[i]; tx++) {
> >> + int ret;
> >> const int idx = ty * r->num_tile_columns + tx;
> >> if (tile_in_slice[idx])
> >> return AVERROR_INVALIDDATA;
> >> tile_in_slice[idx] = true;
> >> ctu_xy(&rx, &ry, tx, ty, pps);
> >> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off, rx, ry,
> >> + ret = pps_add_ctus(pps, off, rx, ry,
> >> r->col_width_val[tx], r->row_height_val[ty]);
> >> + av_assert2(ret >= 0);
> >> + pps->num_ctus_in_slice[i] += ret;
> >> }
> >> }
> >>
> >> @@ -610,8 +599,10 @@ static void pps_no_rect_slice(VVCPPS* pps)
> >>
> >> for (int tile_y = 0; tile_y < r->num_tile_rows; tile_y++) {
> >> for (int tile_x = 0; tile_x < r->num_tile_columns; tile_x++) {
> >> + int ret;
> >> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
> >> - pps_add_ctus(pps, &off, rx, ry, r->col_width_val[tile_x],
> >> r->row_height_val[tile_y]);
> >> + ret = pps_add_ctus(pps, &off, rx, ry,
> >> r->col_width_val[tile_x], r->row_height_val[tile_y]);
> >> + av_assert2(ret >= 0);
> >> }
> >> }
> >> }
> >> --
> >> 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".
> _______________________________________________
> 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".
>
_______________________________________________
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] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level
2025-04-29 13:24 ` Nuo Mi
@ 2025-05-18 13:16 ` Frank Plowman
2025-05-24 10:12 ` Nuo Mi
0 siblings, 1 reply; 6+ messages in thread
From: Frank Plowman @ 2025-05-18 13:16 UTC (permalink / raw)
To: ffmpeg-devel
On 29/04/2025 14:24, Nuo Mi wrote:
> Hi Frank,
> Thank you for the detail.
>
> On Mon, Apr 28, 2025 at 10:35 PM Frank Plowman <post@frankplowman.com>
> wrote:
>
>> On 28/04/2025 14:33, Nuo Mi wrote:
>>> Hi Frank,
>>> Thank you for the v2.
>>> Could we remove all asserts?
>>> Asserts can cause the application to crash at runtime.
>>
>> Hi,
>>
>> I think av_assert2s are the right thing to use here. In case it was not
>> clear, these asserts should never be triggered by any bitstream (legal
>> or illegal). The alternatives, as I see it, are both less favourable:
>>
>> * Don't check the return value at all. If the assumption above that
>> pps_add_ctus shouldn't fail in these cases is incorrect, then all of a
>> sudden there is a rather unscrutable error arising from subtracting a
>> value from off, which might be rather difficult to debug. An assertion
>> is better because it makes the issue obvious by crashing, and
>> immediately points to the location in the code which is problematic.
>>
> FFmpeg will run on multiple computers, so detecting and returning an error
> is better than crashing the program — even if it's a rare occurrence.
>
>>
>> * Add a runtime check for these cases. If the assumption above is
>> correct, then we're incurring needless runtime penalty checking for
>> things which are always true. An av_assert2 is better because it is
>> only enabled in debug builds, and not where performance is essential.
>>
> The entire process happens at the PPS level, and usually, we only have one
> per stream, so the performance loss should be minimal.
>
Ok, I've replaced the asserts with runtime errors in v3.
>>
>>>
>>> On Sun, Apr 27, 2025 at 4:48 PM Frank Plowman <post@frankplowman.com>
>> wrote:
>>>
>>>> In d5dbcc00d889fb17948b025a468b00ddbea9e058, it was hoped that detection
>>>> of subpicture overlaps could be performed at the tile level, so as to
>>>> avoid introducing per-CTU checks. Unfortunately since that patch,
>>>> fuzzing has indicated there are some structures involving
>>>> pps_subpic_one_or_more_tiles_slice where tile-level checking is not
>>>> sufficient. Performing the check at the CTU level should (touch wood)
>>>> be the be-all and and-all of this, as CTUs are the lowest common
>>>> denominator of the picture partitioning.
>>>>
>>>> Signed-off-by: Frank Plowman <post@frankplowman.com>
>>>> ---
>>>> Changes since v1:
>>>> * Merge pps_add_ctus and pps_add_ctus_check
>>>> * Change if/else for early-exit where possible
>>>>
>>>> ---
>>>> libavcodec/vvc/ps.c | 71 ++++++++++++++++++++-------------------------
>>>> 1 file changed, 31 insertions(+), 40 deletions(-)
>>>>
>>>> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
>>>> index e8c312d8ac..ed96268bae 100644
>>>> --- a/libavcodec/vvc/ps.c
>>>> +++ b/libavcodec/vvc/ps.c
>>>> @@ -408,6 +408,8 @@ static int pps_add_ctus(VVCPPS *pps, int *off, const
>>>> int rx, const int ry,
>>>> int start = *off;
>>>> for (int y = 0; y < h; y++) {
>>>> for (int x = 0; x < w; x++) {
>>>> + if (*off >= pps->ctb_count)
>>>> + return AVERROR_INVALIDDATA;
>>>> pps->ctb_addr_in_slice[*off] = ctu_rs(rx + x, ry + y, pps);
>>>> (*off)++;
>>>> }
>>>> @@ -420,9 +422,11 @@ static void pps_single_slice_picture(VVCPPS *pps,
>> int
>>>> *off)
>>>> pps->num_ctus_in_slice[0] = 0;
>>>> for (int j = 0; j < pps->r->num_tile_rows; j++) {
>>>> for (int i = 0; i < pps->r->num_tile_columns; i++) {
>>>> - pps->num_ctus_in_slice[0] += pps_add_ctus(pps, off,
>>>> + const int ret = pps_add_ctus(pps, off,
>>>> pps->col_bd[i], pps->row_bd[j],
>>>> pps->r->col_width_val[i], pps->r->row_height_val[j]);
>>>> + av_assert2(ret >= 0);
>>>> + pps->num_ctus_in_slice[0] += ret;
>>>> }
>>>> }
>>>> }
>>>> @@ -451,50 +455,36 @@ static void subpic_tiles(int *tile_x, int *tile_y,
>>>> int *tile_x_end, int *tile_y_
>>>> (*tile_y_end)++;
>>>> }
>>>>
>>>> -static bool mark_tile_as_used(bool *tile_in_subpic, const int tx, const
>>>> int ty, const int tile_columns)
>>>> -{
>>>> - const size_t tile_idx = ty * tile_columns + tx;
>>>> - if (tile_in_subpic[tile_idx]) {
>>>> - /* the tile is covered by other subpictures */
>>>> - return false;
>>>> - }
>>>> - tile_in_subpic[tile_idx] = true;
>>>> - return true;
>>>> -}
>>>> -
>>>> -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)
>>>> +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)
>>>> {
>>>> - const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] +
>>>> sps->r->sps_subpic_height_minus1[i];
>>>> - const int tile_bottom = pps->row_bd[ty] +
>> pps->r->row_height_val[ty]
>>>> - 1;
>>>> - const bool is_final_subpic_in_tile = subpic_bottom == tile_bottom;
>>>> -
>>>> - if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic,
>> tx,
>>>> ty, pps->r->num_tile_columns))
>>>> - return AVERROR_INVALIDDATA;
>>>> -
>>>> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off,
>>>> + const int ret = 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);
>>>> + if (ret < 0)
>>>> + return ret;
>>>>
>>>> + pps->num_ctus_in_slice[i] = ret;
>>>> return 0;
>>>> }
>>>>
>>>> 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)
>>>> + const int i, int *off)
>>>> {
>>>> for (int ty = tile_y; ty < y_end; ty++) {
>>>> for (int tx = tile_x; tx < x_end; tx++) {
>>>> - if (!mark_tile_as_used(tile_in_subpic, tx, ty,
>>>> pps->r->num_tile_columns))
>>>> - return AVERROR_INVALIDDATA;
>>>> -
>>>> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off,
>>>> + const int ret = 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]);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + pps->num_ctus_in_slice[i] += ret;
>>>> }
>>>> }
>>>> return 0;
>>>> }
>>>>
>>>> -static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int
>> i,
>>>> int *off, bool *tile_in_subpic)
>>>> +static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int
>> i,
>>>> int *off)
>>>> {
>>>> int tx, ty, x_end, y_end;
>>>>
>>>> @@ -503,9 +493,9 @@ static int pps_subpic_slice(VVCPPS *pps, const
>> VVCSPS
>>>> *sps, const int i, int *of
>>>>
>>>> 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])
>>>> - return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty,
>>>> off, tile_in_subpic);
>>>> + return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty,
>>>> off);
>>>> else
>>>> - return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
>>>> y_end, i, off, tile_in_subpic);
>>>> + return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
>>>> y_end, i, off);
>>>> }
>>>>
>>>> static int pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps,
>>>> int *off)
>>>> @@ -513,18 +503,11 @@ 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 {
>>>> - 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);
>>>> + const int ret = pps_subpic_slice(pps, sps, i, off);
>>>> 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;
>>>> }
>>>> @@ -538,9 +521,12 @@ static int pps_one_tile_slices(VVCPPS *pps, const
>> int
>>>> tile_idx, int i, int *off)
>>>> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
>>>> ctu_y_end = ry + r->row_height_val[tile_y];
>>>> while (ry < ctu_y_end) {
>>>> + int ret;
>>>> pps->slice_start_offset[i] = *off;
>>>> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, rx, ry,
>>>> + ret = pps_add_ctus(pps, off, rx, ry,
>>>> r->col_width_val[tile_x], r->slice_height_in_ctus[i]);
>>>> + av_assert2(ret >= 0);
>>>> + pps->num_ctus_in_slice[i] = ret;
>>>> ry += r->slice_height_in_ctus[i++];
>>>> }
>>>> i--;
>>>> @@ -557,13 +543,16 @@ static int pps_multi_tiles_slice(VVCPPS *pps,
>> const
>>>> int tile_idx, const int i, i
>>>> pps->num_ctus_in_slice[i] = 0;
>>>> for (int ty = tile_y; ty <= tile_y +
>>>> r->pps_slice_height_in_tiles_minus1[i]; ty++) {
>>>> for (int tx = tile_x; tx <= tile_x +
>>>> r->pps_slice_width_in_tiles_minus1[i]; tx++) {
>>>> + int ret;
>>>> const int idx = ty * r->num_tile_columns + tx;
>>>> if (tile_in_slice[idx])
>>>> return AVERROR_INVALIDDATA;
>>>> tile_in_slice[idx] = true;
>>>> ctu_xy(&rx, &ry, tx, ty, pps);
>>>> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off, rx, ry,
>>>> + ret = pps_add_ctus(pps, off, rx, ry,
>>>> r->col_width_val[tx], r->row_height_val[ty]);
>>>> + av_assert2(ret >= 0);
>>>> + pps->num_ctus_in_slice[i] += ret;
>>>> }
>>>> }
>>>>
>>>> @@ -610,8 +599,10 @@ static void pps_no_rect_slice(VVCPPS* pps)
>>>>
>>>> for (int tile_y = 0; tile_y < r->num_tile_rows; tile_y++) {
>>>> for (int tile_x = 0; tile_x < r->num_tile_columns; tile_x++) {
>>>> + int ret;
>>>> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
>>>> - pps_add_ctus(pps, &off, rx, ry, r->col_width_val[tile_x],
>>>> r->row_height_val[tile_y]);
>>>> + ret = pps_add_ctus(pps, &off, rx, ry,
>>>> r->col_width_val[tile_x], r->row_height_val[tile_y]);
>>>> + av_assert2(ret >= 0);
>>>> }
>>>> }
>>>> }
>>>> --
>>>> 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".
>> _______________________________________________
>> 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".
>>
> _______________________________________________
> 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".
_______________________________________________
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] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level
2025-05-18 13:16 ` Frank Plowman
@ 2025-05-24 10:12 ` Nuo Mi
0 siblings, 0 replies; 6+ messages in thread
From: Nuo Mi @ 2025-05-24 10:12 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Sun, May 18, 2025 at 9:16 PM Frank Plowman <post@frankplowman.com> wrote:
> On 29/04/2025 14:24, Nuo Mi wrote:
> > Hi Frank,
> > Thank you for the detail.
> >
> > On Mon, Apr 28, 2025 at 10:35 PM Frank Plowman <post@frankplowman.com>
> > wrote:
> >
> >> On 28/04/2025 14:33, Nuo Mi wrote:
> >>> Hi Frank,
> >>> Thank you for the v2.
> >>> Could we remove all asserts?
> >>> Asserts can cause the application to crash at runtime.
> >>
> >> Hi,
> >>
> >> I think av_assert2s are the right thing to use here. In case it was not
> >> clear, these asserts should never be triggered by any bitstream (legal
> >> or illegal). The alternatives, as I see it, are both less favourable:
> >>
> >> * Don't check the return value at all. If the assumption above that
> >> pps_add_ctus shouldn't fail in these cases is incorrect, then all of a
> >> sudden there is a rather unscrutable error arising from subtracting a
> >> value from off, which might be rather difficult to debug. An assertion
> >> is better because it makes the issue obvious by crashing, and
> >> immediately points to the location in the code which is problematic.
> >>
> > FFmpeg will run on multiple computers, so detecting and returning an
> error
> > is better than crashing the program — even if it's a rare occurrence.
> >
> >>
> >> * Add a runtime check for these cases. If the assumption above is
> >> correct, then we're incurring needless runtime penalty checking for
> >> things which are always true. An av_assert2 is better because it is
> >> only enabled in debug builds, and not where performance is essential.
> >>
> > The entire process happens at the PPS level, and usually, we only have
> one
> > per stream, so the performance loss should be minimal.
> >
>
> Ok, I've replaced the asserts with runtime errors in v3.
>
Thank you, Frank,
Applied v3.
>
> >>
> >>>
> >>> On Sun, Apr 27, 2025 at 4:48 PM Frank Plowman <post@frankplowman.com>
> >> wrote:
> >>>
> >>>> In d5dbcc00d889fb17948b025a468b00ddbea9e058, it was hoped that
> detection
> >>>> of subpicture overlaps could be performed at the tile level, so as to
> >>>> avoid introducing per-CTU checks. Unfortunately since that patch,
> >>>> fuzzing has indicated there are some structures involving
> >>>> pps_subpic_one_or_more_tiles_slice where tile-level checking is not
> >>>> sufficient. Performing the check at the CTU level should (touch wood)
> >>>> be the be-all and and-all of this, as CTUs are the lowest common
> >>>> denominator of the picture partitioning.
> >>>>
> >>>> Signed-off-by: Frank Plowman <post@frankplowman.com>
> >>>> ---
> >>>> Changes since v1:
> >>>> * Merge pps_add_ctus and pps_add_ctus_check
> >>>> * Change if/else for early-exit where possible
> >>>>
> >>>> ---
> >>>> libavcodec/vvc/ps.c | 71
> ++++++++++++++++++++-------------------------
> >>>> 1 file changed, 31 insertions(+), 40 deletions(-)
> >>>>
> >>>> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
> >>>> index e8c312d8ac..ed96268bae 100644
> >>>> --- a/libavcodec/vvc/ps.c
> >>>> +++ b/libavcodec/vvc/ps.c
> >>>> @@ -408,6 +408,8 @@ static int pps_add_ctus(VVCPPS *pps, int *off,
> const
> >>>> int rx, const int ry,
> >>>> int start = *off;
> >>>> for (int y = 0; y < h; y++) {
> >>>> for (int x = 0; x < w; x++) {
> >>>> + if (*off >= pps->ctb_count)
> >>>> + return AVERROR_INVALIDDATA;
> >>>> pps->ctb_addr_in_slice[*off] = ctu_rs(rx + x, ry + y,
> pps);
> >>>> (*off)++;
> >>>> }
> >>>> @@ -420,9 +422,11 @@ static void pps_single_slice_picture(VVCPPS *pps,
> >> int
> >>>> *off)
> >>>> pps->num_ctus_in_slice[0] = 0;
> >>>> for (int j = 0; j < pps->r->num_tile_rows; j++) {
> >>>> for (int i = 0; i < pps->r->num_tile_columns; i++) {
> >>>> - pps->num_ctus_in_slice[0] += pps_add_ctus(pps, off,
> >>>> + const int ret = pps_add_ctus(pps, off,
> >>>> pps->col_bd[i], pps->row_bd[j],
> >>>> pps->r->col_width_val[i], pps->r->row_height_val[j]);
> >>>> + av_assert2(ret >= 0);
> >>>> + pps->num_ctus_in_slice[0] += ret;
> >>>> }
> >>>> }
> >>>> }
> >>>> @@ -451,50 +455,36 @@ static void subpic_tiles(int *tile_x, int
> *tile_y,
> >>>> int *tile_x_end, int *tile_y_
> >>>> (*tile_y_end)++;
> >>>> }
> >>>>
> >>>> -static bool mark_tile_as_used(bool *tile_in_subpic, const int tx,
> const
> >>>> int ty, const int tile_columns)
> >>>> -{
> >>>> - const size_t tile_idx = ty * tile_columns + tx;
> >>>> - if (tile_in_subpic[tile_idx]) {
> >>>> - /* the tile is covered by other subpictures */
> >>>> - return false;
> >>>> - }
> >>>> - tile_in_subpic[tile_idx] = true;
> >>>> - return true;
> >>>> -}
> >>>> -
> >>>> -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)
> >>>> +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)
> >>>> {
> >>>> - const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] +
> >>>> sps->r->sps_subpic_height_minus1[i];
> >>>> - const int tile_bottom = pps->row_bd[ty] +
> >> pps->r->row_height_val[ty]
> >>>> - 1;
> >>>> - const bool is_final_subpic_in_tile = subpic_bottom ==
> tile_bottom;
> >>>> -
> >>>> - if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic,
> >> tx,
> >>>> ty, pps->r->num_tile_columns))
> >>>> - return AVERROR_INVALIDDATA;
> >>>> -
> >>>> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off,
> >>>> + const int ret = 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);
> >>>> + if (ret < 0)
> >>>> + return ret;
> >>>>
> >>>> + pps->num_ctus_in_slice[i] = ret;
> >>>> return 0;
> >>>> }
> >>>>
> >>>> 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)
> >>>> + const int i, int *off)
> >>>> {
> >>>> for (int ty = tile_y; ty < y_end; ty++) {
> >>>> for (int tx = tile_x; tx < x_end; tx++) {
> >>>> - if (!mark_tile_as_used(tile_in_subpic, tx, ty,
> >>>> pps->r->num_tile_columns))
> >>>> - return AVERROR_INVALIDDATA;
> >>>> -
> >>>> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off,
> >>>> + const int ret = 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]);
> >>>> + if (ret < 0)
> >>>> + return ret;
> >>>> +
> >>>> + pps->num_ctus_in_slice[i] += ret;
> >>>> }
> >>>> }
> >>>> return 0;
> >>>> }
> >>>>
> >>>> -static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int
> >> i,
> >>>> int *off, bool *tile_in_subpic)
> >>>> +static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int
> >> i,
> >>>> int *off)
> >>>> {
> >>>> int tx, ty, x_end, y_end;
> >>>>
> >>>> @@ -503,9 +493,9 @@ static int pps_subpic_slice(VVCPPS *pps, const
> >> VVCSPS
> >>>> *sps, const int i, int *of
> >>>>
> >>>> 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])
> >>>> - return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx,
> ty,
> >>>> off, tile_in_subpic);
> >>>> + return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx,
> ty,
> >>>> off);
> >>>> else
> >>>> - return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
> >>>> y_end, i, off, tile_in_subpic);
> >>>> + return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end,
> >>>> y_end, i, off);
> >>>> }
> >>>>
> >>>> static int pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS
> *sps,
> >>>> int *off)
> >>>> @@ -513,18 +503,11 @@ 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 {
> >>>> - 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);
> >>>> + const int ret = pps_subpic_slice(pps, sps, i, off);
> >>>> 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;
> >>>> }
> >>>> @@ -538,9 +521,12 @@ static int pps_one_tile_slices(VVCPPS *pps, const
> >> int
> >>>> tile_idx, int i, int *off)
> >>>> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
> >>>> ctu_y_end = ry + r->row_height_val[tile_y];
> >>>> while (ry < ctu_y_end) {
> >>>> + int ret;
> >>>> pps->slice_start_offset[i] = *off;
> >>>> - pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, rx, ry,
> >>>> + ret = pps_add_ctus(pps, off, rx, ry,
> >>>> r->col_width_val[tile_x], r->slice_height_in_ctus[i]);
> >>>> + av_assert2(ret >= 0);
> >>>> + pps->num_ctus_in_slice[i] = ret;
> >>>> ry += r->slice_height_in_ctus[i++];
> >>>> }
> >>>> i--;
> >>>> @@ -557,13 +543,16 @@ static int pps_multi_tiles_slice(VVCPPS *pps,
> >> const
> >>>> int tile_idx, const int i, i
> >>>> pps->num_ctus_in_slice[i] = 0;
> >>>> for (int ty = tile_y; ty <= tile_y +
> >>>> r->pps_slice_height_in_tiles_minus1[i]; ty++) {
> >>>> for (int tx = tile_x; tx <= tile_x +
> >>>> r->pps_slice_width_in_tiles_minus1[i]; tx++) {
> >>>> + int ret;
> >>>> const int idx = ty * r->num_tile_columns + tx;
> >>>> if (tile_in_slice[idx])
> >>>> return AVERROR_INVALIDDATA;
> >>>> tile_in_slice[idx] = true;
> >>>> ctu_xy(&rx, &ry, tx, ty, pps);
> >>>> - pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off, rx,
> ry,
> >>>> + ret = pps_add_ctus(pps, off, rx, ry,
> >>>> r->col_width_val[tx], r->row_height_val[ty]);
> >>>> + av_assert2(ret >= 0);
> >>>> + pps->num_ctus_in_slice[i] += ret;
> >>>> }
> >>>> }
> >>>>
> >>>> @@ -610,8 +599,10 @@ static void pps_no_rect_slice(VVCPPS* pps)
> >>>>
> >>>> for (int tile_y = 0; tile_y < r->num_tile_rows; tile_y++) {
> >>>> for (int tile_x = 0; tile_x < r->num_tile_columns; tile_x++)
> {
> >>>> + int ret;
> >>>> ctu_xy(&rx, &ry, tile_x, tile_y, pps);
> >>>> - pps_add_ctus(pps, &off, rx, ry, r->col_width_val[tile_x],
> >>>> r->row_height_val[tile_y]);
> >>>> + ret = pps_add_ctus(pps, &off, rx, ry,
> >>>> r->col_width_val[tile_x], r->row_height_val[tile_y]);
> >>>> + av_assert2(ret >= 0);
> >>>> }
> >>>> }
> >>>> }
> >>>> --
> >>>> 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".
> >> _______________________________________________
> >> 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".
> >>
> > _______________________________________________
> > 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".
>
>
> _______________________________________________
> 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".
>
_______________________________________________
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] 6+ messages in thread
end of thread, other threads:[~2025-05-24 10:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-27 8:47 [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level Frank Plowman
2025-04-28 13:33 ` Nuo Mi
2025-04-28 14:34 ` Frank Plowman
2025-04-29 13:24 ` Nuo Mi
2025-05-18 13:16 ` Frank Plowman
2025-05-24 10:12 ` Nuo Mi
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