[FFmpeg-devel] [PATCH 1/2] lavc/vvc: Fix slice map construction for small subpics

[FFmpeg-devel] [PATCH 1/2] lavc/vvc: Fix slice map construction for small subpics

[Frank Plowman]

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

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

[Frank Plowman]

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

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

[Nuo Mi]

Hi Frank,
Thank you for the patch.

On Sun, Feb 9, 2025 at 11:45 PM Frank Plowman <post@frankplowman.com> wrote:

> 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 we have  VVC_MAX_TILES_PER_AU rows. this will overwrite.
How about tile_idx = ty * pps->r->num_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);
>
Calculating tiles for each slice smaller than one tile is inefficient.
Maybe we can move subpic_tiles() to pps_single_slice_per_subpic(); this
might simplify the patch.

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

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

[Frank Plowman]

Hi,

Thanks for your review.

On 16/02/2025 15:19, Nuo Mi wrote:
> Hi Frank,
> Thank you for the patch.
> 
> On Sun, Feb 9, 2025 at 11:45 PM Frank Plowman <post@frankplowman.com> wrote:
> 
>> 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 we have  VVC_MAX_TILES_PER_AU rows. this will overwrite.
> How about tile_idx = ty * pps->r->num_tile_columns + tx?
> 

Ah, I'd presumed VVC_MAX_TILE_COLUMNS * VVC_MAX_TILE_ROWS =
VVC_MAX_TILES_PER_AU but I see now this is not the case.  Fixed in v2.

>>
> 
> +        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);
>>
> Calculating tiles for each slice smaller than one tile is inefficient.
> Maybe we can move subpic_tiles() to pps_single_slice_per_subpic(); this
> might simplify the patch.
> 

I don't think this will be all that straightforward.  In
pps_single_slice_per_subpic we are iterating over slices whose locations
and sizes are determined by explicitly-signaled subpictures.  Because
their locations are explicitly-signaled and are therefore pretty much
arbitrary, it will be difficult in pps_single_slice_per_subpic to group
together the slices/subpictures which make up a given tile.  I think
it's possible, but by the time you've implemented the logic to figure
that out, I don't think there would be any performance benefit over just
calling subpic_tiles for each subpic.

It's also worth noting that I don't think there are any encoders out
there which actually produce subpictures smaller than a tile, at least
I've never seen a bitstream which exercises
pps_subpic_less_than_one_tile_slice.  I think changing
pps_single_slice_per_subpic could run the risk of slowing down the much
more common pps_subpic_one_or_more_tiles_slice.

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


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