| Message ID | 20230701013651.30004-2-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/cbs_h266: store SubpicIdVal in the context | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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. > 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". >
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".
On 7/1/2023 7:33 PM, James Almer wrote: > 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. Looking at the spec, it's indeed RowHeightVal and ColWidthVal what's used everywhere, including the couple - 1 where i added them. > > I'll send an updated patch to do the same for ColWidthVal, now that i > notice the same happens with it.
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, 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); } }
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(-)