Thank you for your review.
On 01/01/2025 04:30, Nuo Mi wrote:
> 👍
>
> 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.
Are you sure? Would subpic_top + subpic_height not give the
y-coordinate of the top edge of the subpic below the current one? And
av_clip(a, amin, amax) returns aclip in the range amin <= aclip <= amax,
so this would allow for y-coordinates in the lower subpic and so next
CTU row.
>
>> + 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)
Ah, I didn't realise this. Yes I agree this is cleaner. Would you like
me to send a v3 or can you just use your local changes?
>
>>
>> -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".
_______________________________________________
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".
