| Message ID | 20240825175027.40160-1-post@frankplowman.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2] lavc/vvc: Validate explicit subpic locations | 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 Mon, Aug 26, 2024 at 1:50 AM Frank Plowman <post@frankplowman.com> wrote: > Implement the missing requirements from H.266 (V3) p. 106 on the > position and size of subpictures whose dimensions are provided > explicitly. > Applied this and "[FFmpeg-devel] [PATCH] lavc/vvc: Prevent OOB access in subpic_tiles" Thank you, Frank. > > Signed-off-by: Frank Plowman <post@frankplowman.com> > --- > Changes since v1 (20240824092827.68912-1-post@frankplowman.com): > * Use temporary variables and AV_CEIL_RSHIFT to make bound calculations > more readable. > * Fix bounds on size. The calculated values are minimums, not > maximums as in v1. Additionally, fix an integer overflow in the bound > calculations. > > libavcodec/cbs_h266_syntax_template.c | 49 +++++++++++++++++++++------ > 1 file changed, 38 insertions(+), 11 deletions(-) > > diff --git a/libavcodec/cbs_h266_syntax_template.c > b/libavcodec/cbs_h266_syntax_template.c > index 9c37996947..a8f5af04d0 100644 > --- a/libavcodec/cbs_h266_syntax_template.c > +++ b/libavcodec/cbs_h266_syntax_template.c > @@ -1061,7 +1061,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, > RWContext *rw, > unsigned int ctb_log2_size_y, min_cb_log2_size_y, > min_qt_log2_size_intra_y, min_qt_log2_size_inter_y, > ctb_size_y, max_num_merge_cand, tmp_width_val, > tmp_height_val; > - uint8_t qp_bd_offset; > + uint8_t qp_bd_offset, sub_width_c, sub_height_c; > > static const uint8_t h266_sub_width_c[] = { > 1, 2, 2, 1 > @@ -1089,6 +1089,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, > RWContext *rw, > > u(3, sps_max_sublayers_minus1, 0, VVC_MAX_SUBLAYERS - 1); > u(2, sps_chroma_format_idc, 0, 3); > + sub_width_c = h266_sub_width_c[current->sps_chroma_format_idc]; > + sub_height_c = h266_sub_height_c[current->sps_chroma_format_idc]; > + > u(2, sps_log2_ctu_size_minus5, 0, 3); > ctb_log2_size_y = current->sps_log2_ctu_size_minus5 + 5; > ctb_size_y = 1 << ctb_log2_size_y; > @@ -1110,8 +1113,6 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, > RWContext *rw, > > flag(sps_conformance_window_flag); > if (current->sps_conformance_window_flag) { > - uint8_t sub_width_c = > h266_sub_width_c[current->sps_chroma_format_idc]; > - uint8_t sub_height_c = > h266_sub_height_c[current->sps_chroma_format_idc]; > uint16_t width = current->sps_pic_width_max_in_luma_samples / > sub_width_c; > uint16_t height = current->sps_pic_height_max_in_luma_samples / > sub_height_c; > ue(sps_conf_win_left_offset, 0, width); > @@ -1160,19 +1161,37 @@ 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) > - ubs(wlen, sps_subpic_ctu_top_left_x[i], 1, i); > - else > + if (current->sps_pic_width_max_in_luma_samples > > ctb_size_y) { > + const unsigned 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, > + AV_CEIL_RSHIFT(win_right_edge, > ctb_log2_size_y) - 1, > + 1, i); > + } else > infer(sps_subpic_ctu_top_left_x[i], 0); > if (current->sps_pic_height_max_in_luma_samples > > - ctb_size_y) > - ubs(hlen, sps_subpic_ctu_top_left_y[i], 1, i); > - else > + ctb_size_y) { > + const unsigned 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, > + AV_CEIL_RSHIFT(win_bottom_edge, > ctb_log2_size_y) - 1, > + 1, i); > + } else > infer(sps_subpic_ctu_top_left_y[i], 0); > if (i < current->sps_num_subpics_minus1 && > current->sps_pic_width_max_in_luma_samples > > ctb_size_y) { > - ubs(wlen, sps_subpic_width_minus1[i], 1, i); > + const unsigned int win_left_edge = > + current->sps_conf_win_left_offset * > sub_width_c; > + const unsigned 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] > + ? win_left_edge_ctus - > current->sps_subpic_ctu_top_left_x[i] > + : 0, > + MAX_UINT_BITS(wlen), 1, i); > } else { > infer(sps_subpic_width_minus1[i], > tmp_width_val - > @@ -1181,7 +1200,15 @@ 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) { > - ubs(hlen, sps_subpic_height_minus1[i], 1, i); > + const unsigned int win_top_edge = > + current->sps_conf_win_top_offset * > sub_height_c; > + const unsigned 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] > + ? win_top_edge_ctus - > current->sps_subpic_ctu_top_left_y[i] > + : 0, > + MAX_UINT_BITS(wlen), 1, i); > } else { > infer(sps_subpic_height_minus1[i], > tmp_height_val - > -- > 2.46.0 > >
diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c index 9c37996947..a8f5af04d0 100644 --- a/libavcodec/cbs_h266_syntax_template.c +++ b/libavcodec/cbs_h266_syntax_template.c @@ -1061,7 +1061,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, unsigned int ctb_log2_size_y, min_cb_log2_size_y, min_qt_log2_size_intra_y, min_qt_log2_size_inter_y, ctb_size_y, max_num_merge_cand, tmp_width_val, tmp_height_val; - uint8_t qp_bd_offset; + uint8_t qp_bd_offset, sub_width_c, sub_height_c; static const uint8_t h266_sub_width_c[] = { 1, 2, 2, 1 @@ -1089,6 +1089,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, u(3, sps_max_sublayers_minus1, 0, VVC_MAX_SUBLAYERS - 1); u(2, sps_chroma_format_idc, 0, 3); + sub_width_c = h266_sub_width_c[current->sps_chroma_format_idc]; + sub_height_c = h266_sub_height_c[current->sps_chroma_format_idc]; + u(2, sps_log2_ctu_size_minus5, 0, 3); ctb_log2_size_y = current->sps_log2_ctu_size_minus5 + 5; ctb_size_y = 1 << ctb_log2_size_y; @@ -1110,8 +1113,6 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, flag(sps_conformance_window_flag); if (current->sps_conformance_window_flag) { - uint8_t sub_width_c = h266_sub_width_c[current->sps_chroma_format_idc]; - uint8_t sub_height_c = h266_sub_height_c[current->sps_chroma_format_idc]; uint16_t width = current->sps_pic_width_max_in_luma_samples / sub_width_c; uint16_t height = current->sps_pic_height_max_in_luma_samples / sub_height_c; ue(sps_conf_win_left_offset, 0, width); @@ -1160,19 +1161,37 @@ 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) - ubs(wlen, sps_subpic_ctu_top_left_x[i], 1, i); - else + if (current->sps_pic_width_max_in_luma_samples > ctb_size_y) { + const unsigned 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, + AV_CEIL_RSHIFT(win_right_edge, ctb_log2_size_y) - 1, + 1, i); + } else infer(sps_subpic_ctu_top_left_x[i], 0); if (current->sps_pic_height_max_in_luma_samples > - ctb_size_y) - ubs(hlen, sps_subpic_ctu_top_left_y[i], 1, i); - else + ctb_size_y) { + const unsigned 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, + AV_CEIL_RSHIFT(win_bottom_edge, ctb_log2_size_y) - 1, + 1, i); + } else infer(sps_subpic_ctu_top_left_y[i], 0); if (i < current->sps_num_subpics_minus1 && current->sps_pic_width_max_in_luma_samples > ctb_size_y) { - ubs(wlen, sps_subpic_width_minus1[i], 1, i); + const unsigned int win_left_edge = + current->sps_conf_win_left_offset * sub_width_c; + const unsigned 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] + ? win_left_edge_ctus - current->sps_subpic_ctu_top_left_x[i] + : 0, + MAX_UINT_BITS(wlen), 1, i); } else { infer(sps_subpic_width_minus1[i], tmp_width_val - @@ -1181,7 +1200,15 @@ 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) { - ubs(hlen, sps_subpic_height_minus1[i], 1, i); + const unsigned int win_top_edge = + current->sps_conf_win_top_offset * sub_height_c; + const unsigned 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] + ? win_top_edge_ctus - current->sps_subpic_ctu_top_left_y[i] + : 0, + MAX_UINT_BITS(wlen), 1, i); } else { infer(sps_subpic_height_minus1[i], tmp_height_val -
Implement the missing requirements from H.266 (V3) p. 106 on the position and size of subpictures whose dimensions are provided explicitly. Signed-off-by: Frank Plowman <post@frankplowman.com> --- Changes since v1 (20240824092827.68912-1-post@frankplowman.com): * Use temporary variables and AV_CEIL_RSHIFT to make bound calculations more readable. * Fix bounds on size. The calculated values are minimums, not maximums as in v1. Additionally, fix an integer overflow in the bound calculations. libavcodec/cbs_h266_syntax_template.c | 49 +++++++++++++++++++++------ 1 file changed, 38 insertions(+), 11 deletions(-)