Re: [FFmpeg-devel] [PATCH v9 06/13] vvcdec: add inter prediction

[Nuo Mi <nuomi2021@gmail.com>]

On Mon, Jan 1, 2024 at 11:04 PM Lynne <dev@lynne.ee> wrote:

> Jan 1, 2024, 15:15 by nuomi2021@gmail.com:
>
> > +
> >
> +////////////////////////////////////////////////////////////////////////////////
> > +//
> >
> +////////////////////////////////////////////////////////////////////////////////
> >
>
> What's with these markers?
>
Copied from hevc code, If you perfered, I can remove it.

>
>
> > +static void FUNC(put_pixels)(int16_t *dst,
> > +    const uint8_t *_src, const ptrdiff_t _src_stride,
> > +    const int height, const int8_t *hf, const int8_t *vf, const int
> width)
> > +{
> > +    const pixel *src            = (const pixel *)_src;
> > +    const ptrdiff_t src_stride  = _src_stride / sizeof(pixel);
> > +
> > +    for (int y = 0; y < height; y++) {
> > +        for (int x = 0; x < width; x++)
> > +            dst[x] = src[x] << (14 - BIT_DEPTH);
> > +        src += src_stride;
> > +        dst += MAX_PB_SIZE;
> > +    }
> > +}
> >
>
> av_image_copy(AV_PIX_FMT_GRAY14, input_format)?
>
It's  a copy from hevc.
if we want change, better change it together.

>
> Also, why is the decoder using 14-bits internally rather than expanding
> it to 16-bits? Headroom to avoid overflows?
>
If we check ff_vvc_inter_luma_filters. ff_vvc_inter_chroma_filters, the sum
of each row is 64.
It means inter will use a 6-bits fixed-point multiply. If your coefficient
is 8-bit, the intermediate result is 14-bit (6 + 8)

>
>
> > +
> > +static void FUNC(put_uni_pixels)(uint8_t *_dst, const ptrdiff_t
> _dst_stride,
> > +    const uint8_t *_src, const ptrdiff_t _src_stride, const int height,
> > +     const int8_t *hf, const int8_t *vf, const int width)
> > +{
> > +    const pixel *src            = (const pixel *)_src;
> > +    pixel *dst                  = (pixel *)_dst;
> > +    const ptrdiff_t src_stride  = _src_stride / sizeof(pixel);
> > +    const ptrdiff_t dst_stride  = _dst_stride / sizeof(pixel);
> > +
> > +    for (int y = 0; y < height; y++) {
> > +        memcpy(dst, src, width * sizeof(pixel));
> > +        src += src_stride;
> > +        dst += dst_stride;
> > +    }
> > +}
> >
>
> av_image_copy()?
> Also, this function is identical for all bit depths.
>
>
> > +
> > +enum TxType {
> > +    DCT2,
> > +    DST7,
> > +    DCT8,
> > +    N_TX_TYPE,
> > +};
> > +
> > +enum TxSize {
> > +    TX_SIZE_2,
> > +    TX_SIZE_4,
> > +    TX_SIZE_8,
> > +    TX_SIZE_16,
> > +    TX_SIZE_32,
> > +    TX_SIZE_64,
> > +    N_TX_SIZE,
> > +};
> >
>
> Could you prefix these enums with VVC_?
>
Sure, I can make the change after merging. Tracked with
https://github.com/ffvvc/FFmpeg/issues/180

>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".