Re: [FFmpeg-devel] [PATCH 2/2] avcodec/cbs_h266: store RowHeightVal in the context

[James Almer <jamrial@gmail.com>]

On 7/1/2023 12:10 PM, Nuo Mi wrote:
> On Sat, Jul 1, 2023 at 9:37 AM James Almer <jamrial@gmail.com> wrote:
> 
>> Stop overwriting values from the bitstream array
>> pps_tile_row_height_minus1.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/cbs_h266_syntax_template.c | 27 ++++++++++++++-------------
>>   1 file changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h266_syntax_template.c
>> b/libavcodec/cbs_h266_syntax_template.c
>> index ec2bb1ccc3..625995a2bd 100644
>> --- a/libavcodec/cbs_h266_syntax_template.c
>> +++ b/libavcodec/cbs_h266_syntax_template.c
>> @@ -1779,14 +1779,14 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
>> RWContext *rw,
>>                        "Tile row height(%d) exceeds picture height\n",i);
>>                 return AVERROR_INVALIDDATA;
>>             }
>> +          current->row_height_val[i] =
>> current->pps_tile_row_height_minus1[i] + 1;
>>             remaining_size -= (current->pps_tile_row_height_minus1[i] + 1);
>>           }
>> -        unified_size = (i == 0 ? pic_height_in_ctbs_y :
>> -                        (current->pps_tile_row_height_minus1[i - 1] + 1));
>> +        unified_size = current->pps_tile_row_height_minus1[i - 1] + 1;
>>
>>           while (remaining_size > 0) {
>>               unified_size = FFMIN(remaining_size, unified_size);
>> -            current->pps_tile_row_height_minus1[i] = unified_size - 1;
>> +            current->row_height_val[i] = unified_size;
>>               remaining_size -= unified_size;
>>               i++;
>>           }
>> @@ -1855,17 +1855,17 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
>> RWContext *rw,
>>                       ctu_x += current->pps_tile_column_width_minus1[j] + 1;
>>                   }
>>                   for (j = 0; j < tile_y; j++) {
>> -                    ctu_y += current->pps_tile_row_height_minus1[j] + 1;
>> +                    ctu_y += current->row_height_val[j];
>>                   }
>>                   if (current->pps_slice_width_in_tiles_minus1[i] == 0 &&
>>                       current->pps_slice_height_in_tiles_minus1[i] == 0 &&
>> -                    current->pps_tile_row_height_minus1[tile_y] > 0) {
>> +                    current->row_height_val[tile_y] > 1) {
>>                       int num_slices_in_tile,
>>                           uniform_slice_height, remaining_height_in_ctbs_y;
>>                       remaining_height_in_ctbs_y =
>> -                        current->pps_tile_row_height_minus1[tile_y] + 1;
>> +                        current->row_height_val[tile_y];
>>                       ues(pps_num_exp_slices_in_tile[i],
>> -                        0, current->pps_tile_row_height_minus1[tile_y],
>> 1, i);
>> +                        0, current->row_height_val[tile_y] - 1, 1, i);
>>                       if (current->pps_num_exp_slices_in_tile[i] == 0) {
>>                           num_slices_in_tile = 1;
>>                           slice_top_left_ctu_x[i] = ctu_x;
>> @@ -1875,7 +1875,7 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
>> RWContext *rw,
>>                           for (j = 0; j <
>> current->pps_num_exp_slices_in_tile[i];
>>                                j++) {
>>
>>   ues(pps_exp_slice_height_in_ctus_minus1[i][j], 0,
>> -
>> current->pps_tile_row_height_minus1[tile_y], 2,
>> +                                current->row_height_val[tile_y] - 1, 2,
>>
> The benefit is not so obvious when we need to -1 in multiple places.

It's not about having a benefit, but to stop writing derived values to a 
raw bitstream struct field. And in the end, i also remove a bunch of +1.

What i did not carefully look in the spec is which of these uses 
actually needs pps_tile_row_height_minus1 and which RowHeightVal. I 
assumed all wanted the latter since it's calculated almost immediately 
after the former is read from the bitstream, but maybe you know better.

I'll send an updated patch to do the same for ColWidthVal, now that i 
notice the same happens with it.

> 
>>                                   i, j);
>>                               slice_height_in_ctus =
>>                                   current->
>> @@ -1890,7 +1890,7 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
>> RWContext *rw,
>>                               remaining_height_in_ctbs_y -=
>> slice_height_in_ctus;
>>                           }
>>                           uniform_slice_height = 1 +
>> -                            (j == 0 ?
>> current->pps_tile_row_height_minus1[tile_y] :
>> +                            (j == 0 ? current->row_height_val[tile_y] - 1:
>>
>>   current->pps_exp_slice_height_in_ctus_minus1[i][j-1]);
>>                           while (remaining_height_in_ctbs_y >
>> uniform_slice_height) {
>>                               current->slice_height_in_ctus[i + j] =
>> @@ -1919,7 +1919,7 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
>> RWContext *rw,
>>                            j <=
>> current->pps_slice_height_in_tiles_minus1[i];
>>                            j++) {
>>                           height +=
>> -                           current->pps_tile_row_height_minus1[tile_y +
>> j] + 1;
>> +                           current->row_height_val[tile_y + j];
>>                       }
>>                       current->slice_height_in_ctus[i] = height;
>>
>> @@ -1959,7 +1959,7 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
>> RWContext *rw,
>>                       ctu_x += current->pps_tile_column_width_minus1[j] + 1;
>>                   }
>>                   for (j = 0; j < tile_y; j++) {
>> -                    ctu_y += current->pps_tile_row_height_minus1[j] + 1;
>> +                    ctu_y += current->row_height_val[j];
>>                   }
>>                   slice_top_left_ctu_x[i] = ctu_x;
>>                   slice_top_left_ctu_y[i] = ctu_y;
>> @@ -1972,7 +1972,7 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
>> RWContext *rw,
>>                   for (j = 0; j <=
>> current->pps_slice_height_in_tiles_minus1[i];
>>                        j++) {
>>                       height +=
>> -                        current->pps_tile_row_height_minus1[tile_y + j] +
>> 1;
>> +                        current->row_height_val[tile_y + j];
>>                   }
>>                   current->slice_height_in_ctus[i] = height;
>>
>> @@ -2015,6 +2015,7 @@ static int FUNC(pps) (CodedBitstreamContext *ctx,
>> RWContext *rw,
>>           infer(pps_tile_column_width_minus1[0], pic_width_in_ctbs_y - 1);
>>           infer(pps_num_exp_tile_rows_minus1, 0);
>>           infer(pps_tile_row_height_minus1[0], pic_height_in_ctbs_y - 1);
>> +        infer(row_height_val[0], pic_height_in_ctbs_y);
>>           infer(num_tile_columns, 1);
>>           infer(num_tile_rows, 1);
>>           infer(num_tiles_in_pic, 1);
>> @@ -3037,7 +3038,7 @@ static int FUNC(slice_header) (CodedBitstreamContext
>> *ctx, RWContext *rw,
>>                    current->sh_slice_address +
>>                    current->sh_num_tiles_in_slice_minus1; tile_idx++) {
>>                   tile_y = tile_idx / pps->num_tile_rows;
>> -                height = pps->pps_tile_row_height_minus1[tile_y] + 1;
>> +                height = pps->row_height_val[tile_y];
>>                   num_entry_points += (entropy_sync ? height : 1);
>>               }
>>           }
>> --
>> 2.41.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".