| Message ID | 20241110121314.12540-1-nuomi2021@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/cbs_h266: Fix regression in DVB clip introduced by 93281630a71c06642adfebebb0d4b105a4e02e91 | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
On 10/11/2024 12:13, Nuo Mi wrote: > This commit introduced a regression to VVC_HDR_UHDTV1_OpenGOP_3840x2160_50fps_HLG10_mosaic.ts. > > Root Cause: > The AV_CEIL_RSHIFT(a, b) macro uses bit tricks that work only when -a is a negative value. > However, due to integer promotion rules, this behavior does not extend to the unsigned int type. > > See "6.3.1.1 Boolean, characters, and integers" in the "ISO/IEC 9899" for details. > > Reported-by: Frank Plowman <post@frankplowman.com> > --- > libavcodec/cbs_h266_syntax_template.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c > index b21d07491b..6b2d6534ef 100644 > --- a/libavcodec/cbs_h266_syntax_template.c > +++ b/libavcodec/cbs_h266_syntax_template.c > @@ -1162,7 +1162,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, > for (i = 1; i <= current->sps_num_subpics_minus1; i++) { > if (!current->sps_subpic_same_size_flag) { > if (current->sps_pic_width_max_in_luma_samples > ctb_size_y) { > - const unsigned int win_right_edge = > + const int win_right_edge = > current->sps_pic_width_max_in_luma_samples > - current->sps_conf_win_right_offset * sub_width_c; > us(wlen, sps_subpic_ctu_top_left_x[i], 0, > @@ -1172,7 +1172,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, > infer(sps_subpic_ctu_top_left_x[i], 0); > if (current->sps_pic_height_max_in_luma_samples > > ctb_size_y) { > - const unsigned int win_bottom_edge = > + const int win_bottom_edge = > current->sps_pic_height_max_in_luma_samples > - current->sps_conf_win_bottom_offset * sub_height_c; > us(hlen, sps_subpic_ctu_top_left_y[i], 0, > @@ -1183,9 +1183,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, > if (i < current->sps_num_subpics_minus1 && > current->sps_pic_width_max_in_luma_samples > > ctb_size_y) { > - const unsigned int win_left_edge = > + const int win_left_edge = > current->sps_conf_win_left_offset * sub_width_c; > - const unsigned int win_left_edge_ctus = > + const int win_left_edge_ctus = > AV_CEIL_RSHIFT(win_left_edge, ctb_log2_size_y); > us(wlen, sps_subpic_width_minus1[i], > win_left_edge_ctus > current->sps_subpic_ctu_top_left_x[i] > @@ -1200,9 +1200,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, > if (i < current->sps_num_subpics_minus1 && > current->sps_pic_height_max_in_luma_samples > > ctb_size_y) { > - const unsigned int win_top_edge = > + const int win_top_edge = > current->sps_conf_win_top_offset * sub_height_c; > - const unsigned int win_top_edge_ctus = > + const int win_top_edge_ctus = > AV_CEIL_RSHIFT(win_top_edge, ctb_log2_size_y); > us(hlen, sps_subpic_height_minus1[i], > win_top_edge_ctus > current->sps_subpic_ctu_top_left_y[i] LGTM!
On Mon, Nov 11, 2024 at 4:27 AM Frank Plowman <post@frankplowman.com> wrote: > > > On 10/11/2024 12:13, Nuo Mi wrote: > > This commit introduced a regression to > VVC_HDR_UHDTV1_OpenGOP_3840x2160_50fps_HLG10_mosaic.ts. > > > > Root Cause: > > The AV_CEIL_RSHIFT(a, b) macro uses bit tricks that work only when -a is > a negative value. > > However, due to integer promotion rules, this behavior does not extend > to the unsigned int type. > > > > See "6.3.1.1 Boolean, characters, and integers" in the "ISO/IEC 9899" > for details. > > > > Reported-by: Frank Plowman <post@frankplowman.com> > > --- > > libavcodec/cbs_h266_syntax_template.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/libavcodec/cbs_h266_syntax_template.c > b/libavcodec/cbs_h266_syntax_template.c > > index b21d07491b..6b2d6534ef 100644 > > --- a/libavcodec/cbs_h266_syntax_template.c > > +++ b/libavcodec/cbs_h266_syntax_template.c > > @@ -1162,7 +1162,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, > RWContext *rw, > > for (i = 1; i <= current->sps_num_subpics_minus1; i++) { > > if (!current->sps_subpic_same_size_flag) { > > if (current->sps_pic_width_max_in_luma_samples > > ctb_size_y) { > > - const unsigned int win_right_edge = > > + const int win_right_edge = > > current->sps_pic_width_max_in_luma_samples > > - current->sps_conf_win_right_offset * > sub_width_c; > > us(wlen, sps_subpic_ctu_top_left_x[i], 0, > > @@ -1172,7 +1172,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, > RWContext *rw, > > infer(sps_subpic_ctu_top_left_x[i], 0); > > if (current->sps_pic_height_max_in_luma_samples > > > ctb_size_y) { > > - const unsigned int win_bottom_edge = > > + const int win_bottom_edge = > > current->sps_pic_height_max_in_luma_samples > > - current->sps_conf_win_bottom_offset * > sub_height_c; > > us(hlen, sps_subpic_ctu_top_left_y[i], 0, > > @@ -1183,9 +1183,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, > RWContext *rw, > > if (i < current->sps_num_subpics_minus1 && > > current->sps_pic_width_max_in_luma_samples > > > ctb_size_y) { > > - const unsigned int win_left_edge = > > + const int win_left_edge = > > current->sps_conf_win_left_offset * > sub_width_c; > > - const unsigned int win_left_edge_ctus = > > + const int win_left_edge_ctus = > > AV_CEIL_RSHIFT(win_left_edge, > ctb_log2_size_y); > > us(wlen, sps_subpic_width_minus1[i], > > win_left_edge_ctus > > current->sps_subpic_ctu_top_left_x[i] > > @@ -1200,9 +1200,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, > RWContext *rw, > > if (i < current->sps_num_subpics_minus1 && > > current->sps_pic_height_max_in_luma_samples > > > ctb_size_y) { > > - const unsigned int win_top_edge = > > + const int win_top_edge = > > current->sps_conf_win_top_offset * > sub_height_c; > > - const unsigned int win_top_edge_ctus = > > + const int win_top_edge_ctus = > > AV_CEIL_RSHIFT(win_top_edge, > ctb_log2_size_y); > > us(hlen, sps_subpic_height_minus1[i], > > win_top_edge_ctus > > current->sps_subpic_ctu_top_left_y[i] > > LGTM! > Thank you. Frank. Merged and backported to 7.1 with help from James > _______________________________________________ > 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". >
diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c index b21d07491b..6b2d6534ef 100644 --- a/libavcodec/cbs_h266_syntax_template.c +++ b/libavcodec/cbs_h266_syntax_template.c @@ -1162,7 +1162,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, for (i = 1; i <= current->sps_num_subpics_minus1; i++) { if (!current->sps_subpic_same_size_flag) { if (current->sps_pic_width_max_in_luma_samples > ctb_size_y) { - const unsigned int win_right_edge = + const int win_right_edge = current->sps_pic_width_max_in_luma_samples - current->sps_conf_win_right_offset * sub_width_c; us(wlen, sps_subpic_ctu_top_left_x[i], 0, @@ -1172,7 +1172,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, infer(sps_subpic_ctu_top_left_x[i], 0); if (current->sps_pic_height_max_in_luma_samples > ctb_size_y) { - const unsigned int win_bottom_edge = + const int win_bottom_edge = current->sps_pic_height_max_in_luma_samples - current->sps_conf_win_bottom_offset * sub_height_c; us(hlen, sps_subpic_ctu_top_left_y[i], 0, @@ -1183,9 +1183,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, if (i < current->sps_num_subpics_minus1 && current->sps_pic_width_max_in_luma_samples > ctb_size_y) { - const unsigned int win_left_edge = + const int win_left_edge = current->sps_conf_win_left_offset * sub_width_c; - const unsigned int win_left_edge_ctus = + const int win_left_edge_ctus = AV_CEIL_RSHIFT(win_left_edge, ctb_log2_size_y); us(wlen, sps_subpic_width_minus1[i], win_left_edge_ctus > current->sps_subpic_ctu_top_left_x[i] @@ -1200,9 +1200,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, if (i < current->sps_num_subpics_minus1 && current->sps_pic_height_max_in_luma_samples > ctb_size_y) { - const unsigned int win_top_edge = + const int win_top_edge = current->sps_conf_win_top_offset * sub_height_c; - const unsigned int win_top_edge_ctus = + const int win_top_edge_ctus = AV_CEIL_RSHIFT(win_top_edge, ctb_log2_size_y); us(hlen, sps_subpic_height_minus1[i], win_top_edge_ctus > current->sps_subpic_ctu_top_left_y[i]