[FFmpeg-devel] [PATCH 3/6] lavc/vvc: Store MIP information over entire CU area

Nuo Mi nuomi2021 at gmail.com
Tue Dec 3 12:04:59 EET 2024


Hi Frank,
Thank you for the patch

On Sat, Nov 30, 2024 at 8:13 PM Frank Plowman <post at frankplowman.com> wrote:

> On 30/11/2024 06:46, Nuo Mi wrote:
> > On Fri, Nov 29, 2024 at 6:19 AM Frank Plowman <post at frankplowman.com>
> wrote:
> >
> >> Previously, the code only stored the MIP mode and transpose flag in the
> >> relevant tables at the top-left corner of the CU.  This information ends
> >> up being retrieved in ff_vvc_intra_pred_* not based on the CU position
> >> but instead the transform unit position (specifically, using the x0 and
> >> y0 from get_luma_predict_unit).  There might be multiple transform units
> >> in a CU, hence the top-left corner of the transform unit might not
> >> coincide with the top-left corner of the CU.  Consequently, we need to
> >> store the MIP information at all positions in the CU, not only its
> >> top-left corner, as we already do for the MIP flag.
> >>
> >> Signed-off-by: Frank Plowman <post at frankplowman.com>
> >> ---
> >>  libavcodec/vvc/ctu.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c
> >> index 1e06119cfd..0030938cf5 100644
> >> --- a/libavcodec/vvc/ctu.c
> >> +++ b/libavcodec/vvc/ctu.c
> >> @@ -975,8 +975,8 @@ static void intra_luma_pred_modes(VVCLocalContext
> *lc)
> >>              for (int y = 0; y < (cb_height>>log2_min_cb_size); y++) {
> >>                  int width = cb_width>>log2_min_cb_size;
> >>                  memset(&fc->tab.imf[x],  cu->intra_mip_flag, width);
> >> -                fc->tab.imtf[x] = intra_mip_transposed_flag;
> >> -                fc->tab.imm[x]  = intra_mip_mode;
> >> +                memset(&fc->tab.imtf[x], intra_mip_transposed_flag,
> >> width);
> >> +                memset(&fc->tab.imm[x], intra_mip_mode, width);
> >
> >  intra_mip_mode is 4 bits, 2 flags are 2 bits. maybe we can use a uint8_t
> > for 3 fields,
> > We only need 1 memset and save 2/3 memory.
>
> I've implemented this (patch attached, to be applied atop the set), but
> it's not as straightforward as it may seem.  In particular, because the
> tables are read directly from when determining which CABAC context to
> use for these flags, we have to add quite a lot of extra code in cabac.c
> to support this special case where the MIP information is a bit field.
> In my implementation, this was done by adding this coerce_to_bool
> parameter to get_inc and get_top.  This does actually save a moderate
> amount of memory though, ~1MB for 4K and ~256kB for 1080p.
>
A coding block can be as small as 4x4, so for a 1080p frame, the memory
required is approximately  2*1920*1080/(4*4) ~= 256 kB.
However, with a maximum delay of 16 frames, the total memory usage will be
256kB * 16=4 MB.

In your patch, coerce_to_bool is set to 1 only when we are in
ff_vvc_intra_mip_flag.
How about defining get_mip_inc like this? This way, we can avoid changing
get_inc

static av_always_inline
uint8_t get_mip_inc (VVCLocalContext *lc, const uint8_t *ctx)
{
    uint8_t left = 0, top = 0;
    get_left_top(lc, &left, &top, lc->cu->x0, lc->cu->y0, ctx, ctx);
    return !!left + !!top;
}

BTW: Using pack_mip_info/unpack_mip_info might be shorter and more concise
than structure_mip_info/destructure_mip_info.

>
> >
> >>
> >
> >                  x += pps->min_cb_width;
> >>              }
> >>              cu->intra_pred_mode_y = intra_mip_mode;
> >> --
> >> 2.47.0
> >>
>
>
> --
> Frank
>


More information about the ffmpeg-devel mailing list