| Message ID | 20240402214843.474910-1-post@frankplowman.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavc/vvc: Only read split_cu_flag if a split is allowed | 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 02/04/2024 22:48, Frank Plowman wrote: > Add a check to ensure some split is possible before reading the > split_cu_flag. This is present in the spec, in VVCv3 section 7.3.11.4. > Its omission could lead to infinite loops and ultimately crashing due to > stack overflow. > --- > libavcodec/vvc/vvc_ctu.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c > index 8ba12c8d9f..32d8bc8f5c 100644 > --- a/libavcodec/vvc/vvc_ctu.c > +++ b/libavcodec/vvc/vvc_ctu.c > @@ -2095,6 +2095,7 @@ static int hls_coding_tree(VVCLocalContext *lc, > const int ch_type = tree_type_curr == DUAL_TREE_CHROMA; > int ret; > VVCAllowedSplit allowed; > + int split_cu_flag; > > if (pps->r->pps_cu_qp_delta_enabled_flag && qg_on_y && cb_sub_div <= sh->cu_qp_delta_subdiv) { > lc->parse.is_cu_qp_delta_coded = 0; > @@ -2109,7 +2110,11 @@ static int hls_coding_tree(VVCLocalContext *lc, > > can_split(lc, x0, y0, cb_width, cb_height, mtt_depth, depth_offset, part_idx, > last_split_mode, tree_type_curr, mode_type_curr, &allowed); > - if (ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height, ch_type, &allowed)) { > + if (allowed.btv || allowed.bth || allowed.ttv || allowed.tth || allowed.qt) > + split_cu_flag = ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height, ch_type, &allowed); > + else > + split_cu_flag = 0; > + if (split_cu_flag) { > VVCSplitMode split = ff_vvc_split_mode(lc, x0, y0, cb_width, cb_height, cqt_depth, mtt_depth, ch_type, &allowed); > VVCModeType mode_type = mode_type_decode(lc, x0, y0, cb_width, cb_height, split, ch_type, mode_type_curr); > Retracting this patch as I missed that this logic is in fact implemented, just elsewhere. There is still a bug here, but it seems the condition to trigger it is more complex that I thought. Should have an alternative patch soon.
On Wed, Apr 3, 2024 at 5:59 PM Frank Plowman <post@frankplowman.com> wrote: > On 02/04/2024 22:48, Frank Plowman wrote: > > Add a check to ensure some split is possible before reading the > > split_cu_flag. This is present in the spec, in VVCv3 section 7.3.11.4. > > Its omission could lead to infinite loops and ultimately crashing due to > > stack overflow. > > --- > > libavcodec/vvc/vvc_ctu.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c > > index 8ba12c8d9f..32d8bc8f5c 100644 > > --- a/libavcodec/vvc/vvc_ctu.c > > +++ b/libavcodec/vvc/vvc_ctu.c > > @@ -2095,6 +2095,7 @@ static int hls_coding_tree(VVCLocalContext *lc, > > const int ch_type = tree_type_curr == > DUAL_TREE_CHROMA; > > int ret; > > VVCAllowedSplit allowed; > > + int split_cu_flag; > > > > if (pps->r->pps_cu_qp_delta_enabled_flag && qg_on_y && cb_sub_div > <= sh->cu_qp_delta_subdiv) { > > lc->parse.is_cu_qp_delta_coded = 0; > > @@ -2109,7 +2110,11 @@ static int hls_coding_tree(VVCLocalContext *lc, > > > > can_split(lc, x0, y0, cb_width, cb_height, mtt_depth, > depth_offset, part_idx, > > last_split_mode, tree_type_curr, mode_type_curr, &allowed); > > - if (ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height, ch_type, > &allowed)) { > > + if (allowed.btv || allowed.bth || allowed.ttv || allowed.tth || > allowed.qt) > > + split_cu_flag = ff_vvc_split_cu_flag(lc, x0, y0, cb_width, > cb_height, ch_type, &allowed); > > + else > > + split_cu_flag = 0; > > + if (split_cu_flag) { > > VVCSplitMode split = ff_vvc_split_mode(lc, x0, y0, > cb_width, cb_height, cqt_depth, mtt_depth, ch_type, &allowed); > > VVCModeType mode_type = mode_type_decode(lc, x0, y0, > cb_width, cb_height, split, ch_type, mode_type_curr); > > > > Retracting this patch as I missed that this logic is in fact > implemented, just elsewhere. There is still a bug here, but it seems > the condition to trigger it is more complex that I thought. Should have > an alternative patch soon. > Hi Frank, Thanks for your patch. Please forward the clip to me as well. I'll use it to test your patch during the review. > _______________________________________________ > 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/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c index 8ba12c8d9f..32d8bc8f5c 100644 --- a/libavcodec/vvc/vvc_ctu.c +++ b/libavcodec/vvc/vvc_ctu.c @@ -2095,6 +2095,7 @@ static int hls_coding_tree(VVCLocalContext *lc, const int ch_type = tree_type_curr == DUAL_TREE_CHROMA; int ret; VVCAllowedSplit allowed; + int split_cu_flag; if (pps->r->pps_cu_qp_delta_enabled_flag && qg_on_y && cb_sub_div <= sh->cu_qp_delta_subdiv) { lc->parse.is_cu_qp_delta_coded = 0; @@ -2109,7 +2110,11 @@ static int hls_coding_tree(VVCLocalContext *lc, can_split(lc, x0, y0, cb_width, cb_height, mtt_depth, depth_offset, part_idx, last_split_mode, tree_type_curr, mode_type_curr, &allowed); - if (ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height, ch_type, &allowed)) { + if (allowed.btv || allowed.bth || allowed.ttv || allowed.tth || allowed.qt) + split_cu_flag = ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height, ch_type, &allowed); + else + split_cu_flag = 0; + if (split_cu_flag) { VVCSplitMode split = ff_vvc_split_mode(lc, x0, y0, cb_width, cb_height, cqt_depth, mtt_depth, ch_type, &allowed); VVCModeType mode_type = mode_type_decode(lc, x0, y0, cb_width, cb_height, split, ch_type, mode_type_curr);