[FFmpeg-devel] [PATCH] lavc/vvc: Fix race condition for MVs cropped to subpic

[Nuo Mi]

Hi Frank,
Thank you for the patch.

On Sun, Dec 22, 2024 at 9:40 PM Frank Plowman <post at frankplowman.com> wrote:

> When the current subpicture has sps_subpic_treated_as_pic_flag equal to
> 1, motion vectors are cropped such that they cannot point to other
> subpictures.  This was accounted for in the prediction logic, but not
> in pred_get_y, which is used by the scheduling logic to determine which
> parts of the reference pictures must have been reconstructed before
> inter prediction of a subsequent frame may begin.  Consequently, where a
> motion vector pointed to a location significantly above the current
> subpicture, there was the possibility of a race condition.  Patch fixes
> this by cropping the motion vector to the current subpicture in
> pred_get_y.
>
> Signed-off-by: Frank Plowman <post at frankplowman.com>
> ---
> You can reproduce this issue with the bitstream available here:
>
> https://chromium.googlesource.com/chromium/src/+/lkgr/media/test/data/vvc_frames_with_ltr.vvc?format=TEXT
> Note this link downloads the file in base 64 format rather than binary.
> You can use base64 -d <BASE 64 FILE> > <BINARY FILE> to convert it to
> binary.  The issue does not reproduce very reliably with a normal number
> of threads.  For better reproducibility, run ffmpeg with 100+ threads.
> ---
>  libavcodec/vvc/ctu.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c
> index f80bce637c..6857c79f2b 100644
> --- a/libavcodec/vvc/ctu.c
> +++ b/libavcodec/vvc/ctu.c
> @@ -2393,23 +2393,30 @@ static int has_inter_luma(const CodingUnit *cu)
>      return cu->pred_mode != MODE_INTRA && cu->pred_mode != MODE_PLT &&
> cu->tree_type != DUAL_TREE_CHROMA;
>  }
>
> -static int pred_get_y(const int y0, const Mv *mv, const int height)
> +static int pred_get_y(const VVCLocalContext *lc, const int y0, const Mv
> *mv, const int height, const VVCFrame *src_frame)
>  {
> -    return FFMAX(0, y0 + (mv->y >> 4) + height);
> +    const VVCSPS *sps = src_frame->sps;
> +    const VVCPPS *pps = src_frame->pps;
> +    const int subpic_idx = lc->sc->sh.r->curr_subpic_idx;
> +    const int subpic_t = pps->subpic_y[subpic_idx];
>
_t reserved by types, line int8_t

> +    const int subpic_treated_as_pic =
> sps->r->sps_subpic_treated_as_pic_flag[subpic_idx];
> +    return FFMAX(subpic_treated_as_pic ? subpic_t : 0, y0 + (mv->y >> 4)
> + height);
>
Clipping to the subpicture bottom might be better, as y + (mv->y >> 4) +
height could be much smaller than the bottom boundary.

>  }
>
> -static void cu_get_max_y(const CodingUnit *cu, int
> max_y[2][VVC_MAX_REF_ENTRIES], const VVCFrameContext *fc)
> +static void cu_get_max_y(const VVCLocalContext *lc, const CodingUnit *cu,
> int max_y[2][VVC_MAX_REF_ENTRIES])
>  {
> +    const VVCFrameContext *fc   = lc->fc;
>      const PredictionUnit *pu    = &cu->pu;
>
>      if (pu->merge_gpm_flag) {
>          for (int i = 0; i < FF_ARRAY_ELEMS(pu->gpm_mv); i++) {
> -            const MvField *mvf  = pu->gpm_mv + i;
> -            const int lx        = mvf->pred_flag - PF_L0;
> -            const int idx       = mvf->ref_idx[lx];
> -            const int y         = pred_get_y(cu->y0, mvf->mv + lx,
> cu->cb_height);
> +            const MvField *mvf    = pu->gpm_mv + i;
> +            const int lx          = mvf->pred_flag - PF_L0;
> +            const int idx         = mvf->ref_idx[lx];
> +            const VVCRefPic *refp = lc->sc->rpl[lx].refs + idx;
> +            const int y           = pred_get_y(lc, cu->y0, mvf->mv + lx,
> cu->cb_height, refp->ref);
>
> -            max_y[lx][idx]      = FFMAX(max_y[lx][idx], y);
> +            max_y[lx][idx]        = FFMAX(max_y[lx][idx], y);

         }
>      } else {
>          const MotionInfo *mi    = &pu->mi;
> @@ -2424,8 +2431,9 @@ static void cu_get_max_y(const CodingUnit *cu, int
> max_y[2][VVC_MAX_REF_ENTRIES]
>                  for (int lx = 0; lx < 2; lx++) {
>                      const PredFlag mask = 1 << lx;
>                      if (mvf->pred_flag & mask) {
> -                        const int idx   = mvf->ref_idx[lx];
> -                        const int y     = pred_get_y(y0, mvf->mv + lx,
> sbh);
> +                        const int idx         = mvf->ref_idx[lx];
> +                        const VVCRefPic *refp = lc->sc->rpl[lx].refs +
> idx;
> +                        int y                 = pred_get_y(lc, y0,
> mvf->mv + lx, sbh, refp->ref);
>
>                          max_y[lx][idx]  = FFMAX(max_y[lx][idx], y +
> max_dmvr_off);
>                      }
> @@ -2452,7 +2460,7 @@ static void ctu_get_pred(VVCLocalContext *lc, const
> int rs)
>
>      while (cu) {
>          if (has_inter_luma(cu)) {
> -            cu_get_max_y(cu, ctu->max_y, fc);
> +            cu_get_max_y(lc, cu, ctu->max_y);
>              ctu->has_dmvr |= cu->pu.dmvr_flag;
>          }
>          cu = cu->next;
> --
> 2.47.0
>
>