Re: [FFmpeg-devel] [PATCH] lavc/vvc: Only read split_cu_flag if a split is allowed

2024-04-03 Thread Nuo Mi
On Wed, Apr 3, 2024 at 5:59 PM Frank Plowman  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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] lavc/vvc: Only read split_cu_flag if a split is allowed

2024-04-03 Thread Frank Plowman

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.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".