[FFmpeg-devel] [PATCH v2] lavc/vvc: Fix NumEntryPoints derivation

[Nuo Mi]

On Sun, Mar 9, 2025 at 9:57 PM Nuo Mi <nuomi2021 at gmail.com> wrote:

>
>
> On Thu, Mar 6, 2025 at 2:40 AM Frank Plowman <post at frankplowman.com>
> wrote:
>
>> If pps_single_slice_per_subpic_flag is 1,
>> slice_{width,height}_in_tiles are undefined and we must instead get the
>> dimensions of the slice by referring to the corresponding subpicture.
>>
>> Signed-off-by: Frank Plowman <post at frankplowman.com>
>> ---
>> Changes since v1:
>> * slice_height_in_tiles is also undefined if
>>   pps->pps_single_slice_per_subpic_flag is 1,
>>   so also derive that from the subpic info.
>> ---
>>  libavcodec/cbs_h266_syntax_template.c | 49 +++++++++++++++++++++++----
>>  1 file changed, 43 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h266_syntax_template.c
>> b/libavcodec/cbs_h266_syntax_template.c
>> index 58211026bb..05634bc19c 100644
>> --- a/libavcodec/cbs_h266_syntax_template.c
>> +++ b/libavcodec/cbs_h266_syntax_template.c
>> @@ -3440,13 +3440,50 @@ static int FUNC(slice_header)
>> (CodedBitstreamContext *ctx, RWContext *rw,
>>              for (i = 0; i < current->curr_subpic_idx; i++) {
>>                  slice_idx += pps->num_slices_in_subpic[i];
>>              }
>> -            width_in_tiles =
>> -                pps->pps_slice_width_in_tiles_minus1[slice_idx] + 1;
>>
>> -            if (entropy_sync)
>> -                height = pps->slice_height_in_ctus[slice_idx];
>> -            else
>> -                height =
>> pps->pps_slice_height_in_tiles_minus1[slice_idx] + 1;
>> +            if (pps->pps_single_slice_per_subpic_flag) {
>> +                const int width_in_ctus =
>> sps->sps_subpic_width_minus1[slice_idx] + 1;
>> +                const int subpic_l =
>> sps->sps_subpic_ctu_top_left_x[slice_idx];
>> +                const int subpic_r = subpic_l + width_in_ctus;
>> +
>> +                int ctb_x = 0, tile_x = 0;
>> +                for (; ctb_x < subpic_l && tile_x <
>> pps->num_tile_columns; tile_x++)
>> +                    ctb_x += pps->col_width_val[tile_x];
>> +
>> +                width_in_tiles = 0;
>> +                for (; ctb_x < subpic_r && tile_x <
>> pps->num_tile_columns; tile_x++) {
>> +                    ctb_x += pps->col_width_val[tile_x];
>> +                    width_in_tiles++;
>> +                }
>> +
>> +                if (entropy_sync)
>> +                    height = sps->sps_subpic_height_minus1[slice_idx] +
>> 1;
>> +                else {
>> +                    const int height_in_ctus =
>> sps->sps_subpic_height_minus1[slice_idx] + 1;
>> +                    const int subpic_t =
>> sps->sps_subpic_ctu_top_left_y[slice_idx];
>> +                    const int subpic_b = subpic_t + height_in_ctus;
>> +
>> +                    int ctb_y = 0, tile_y = 0, height_in_tiles;
>> +                    for (; ctb_y < subpic_t && tile_y <
>> pps->num_tile_rows; tile_y++)
>> +                        ctb_y += pps->row_height_val[tile_y];
>> +
>> +                    height_in_tiles = 0;
>> +                    for (; ctb_y < subpic_b && tile_y <
>> pps->num_tile_rows; tile_y++) {
>> +                        ctb_y += pps->row_height_val[tile_y];
>> +                        height_in_tiles++;
>> +                    }
>>
> Hi Frank,
> Thank you for the v2.
> Same logic as width_in_tiles, with only a few different parameters. Better
> to avoid duplication.
> How about using a single function: size_in_tiles(size_in_ctu, top_left,
> tile_val, num_tile)?
>
Hmm, it's in CBS. Sorry, you can ignore the suggestion.
There's a lot of duplicate code in CBS—I tried to refactor it into
functions, but Mark objected
Will push this version later.

> +
>> +                    height = height_in_tiles;
>> +                }
>> +            } else {
>> +                width_in_tiles =
>> +                    pps->pps_slice_width_in_tiles_minus1[slice_idx] + 1;
>> +
>> +                if (entropy_sync)
>> +                    height = pps->slice_height_in_ctus[slice_idx];
>> +                else
>> +                    height =
>> pps->pps_slice_height_in_tiles_minus1[slice_idx] + 1;
>> +            }
>>
>>              current->num_entry_points = width_in_tiles * height;
>>          } else {
>> --
>> 2.47.0
>>
>>