👍 On Tue, Dec 31, 2024 at 2:02 AM Frank Plowman <p...@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 <p...@frankplowman.com> > --- > Changes since v1: > * Also clip max_y to the subpic_bottom, in order to prevent adding > unecessary dependencies. max_y is also clipped to the picture bottom > where it wasn't before. This doesn't make any difference to the > function of the code as is, but this was only previously unnecessary > due to a detail of how y-coordinates are mapped to CTU rows. To aid > comprehensibility, the MV is now clipped to both the top and bottom edges > of the picture. > * Rename subpic_t to subpic_top to avoid clashing with conventions for > type names. > --- > libavcodec/vvc/ctu.c | 34 +++++++++++++++++++++++----------- > 1 file changed, 23 insertions(+), 11 deletions(-) > > diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c > index f80bce637c..b82dece0dd 100644 > --- a/libavcodec/vvc/ctu.c > +++ b/libavcodec/vvc/ctu.c > @@ -2393,23 +2393,34 @@ 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 pic_height = pps->height; > + const int subpic_idx = lc->sc->sh.r->curr_subpic_idx; > + const int subpic_top = pps->subpic_y[subpic_idx]; > + const int subpic_bottom = subpic_top + pps->subpic_height[subpic_idx] > - 1; > -1 is not needed. > + const int subpic_treated_as_pic = > sps->r->sps_subpic_treated_as_pic_flag[subpic_idx]; > + const int lower_bound = subpic_treated_as_pic ? subpic_top : 0; > + const int upper_bound = subpic_treated_as_pic ? subpic_bottom : > pic_height; > + return av_clip(y0 + (mv->y >> 4) + height, lower_bound, upper_bound); > } > This can be further simplified as static int pred_get_y(const VVCLocalContext *lc, const int y0, const Mv *mv, const int height, const VVCFrame *src_frame) { const VVCPPS *pps = src_frame->pps; const int subpic_idx = lc->sc->sh.r->curr_subpic_idx; const int top = pps->subpic_y[subpic_idx]; const int bottom = top + pps->subpic_height[subpic_idx]; return av_clip(y0 + (mv->y >> 4) + height, top, bottom); } since subpic_y and subpic_height have valid values even if we have no subpic. see static void pps_subpic(VVCPPS *pps, const VVCSPS *sps) > > -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 +2435,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 +2464,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 > > _______________________________________________ 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".