[FFmpeg-devel] [PATCH 2/4] lavc/mpegvideo: use H263DSP dequant function

[James Almer]

On 6/12/2024 1:47 AM, Rémi Denis-Courmont wrote:
> ---
>   configure              |  4 ++--
>   libavcodec/mpegvideo.c | 46 +++++++++++-------------------------------
>   2 files changed, 14 insertions(+), 36 deletions(-)
> 
> diff --git a/configure b/configure
> index 6baa9b0646..eb9d1b1f5d 100755
> --- a/configure
> +++ b/configure
> @@ -2957,8 +2957,8 @@ ftr_decoder_select="adts_header"
>   g2m_decoder_deps="zlib"
>   g2m_decoder_select="blockdsp idctdsp jpegtables"
>   g729_decoder_select="audiodsp"
> -h261_decoder_select="mpegvideodec"
> -h261_encoder_select="mpegvideoenc"
> +h261_decoder_select="h263dsp mpegvideodec"
> +h261_encoder_select="h263dsp mpegvideoenc"
>   h263_decoder_select="h263_parser h263dsp mpegvideodec qpeldsp"
>   h263_encoder_select="h263dsp mpegvideoenc"
>   h263i_decoder_select="h263_decoder"
> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> index 7af823b8bd..b35fd37083 100644
> --- a/libavcodec/mpegvideo.c
> +++ b/libavcodec/mpegvideo.c
> @@ -201,13 +201,11 @@ static void dct_unquantize_mpeg2_inter_c(MpegEncContext *s,
>   static void dct_unquantize_h263_intra_c(MpegEncContext *s,
>                                     int16_t *block, int n, int qscale)
>   {
> -    int i, level, qmul, qadd;
> -    int nCoeffs;
> +    int qmul = qscale << 1;
> +    int qadd, nCoeffs;
>   
>       av_assert2(s->block_last_index[n]>=0 || s->h263_aic);
>   
> -    qmul = qscale << 1;
> -
>       if (!s->h263_aic) {
>           block[0] *= n < 4 ? s->y_dc_scale : s->c_dc_scale;
>           qadd = (qscale - 1) | 1;
> @@ -215,47 +213,24 @@ static void dct_unquantize_h263_intra_c(MpegEncContext *s,
>           qadd = 0;
>       }
>       if(s->ac_pred)
> -        nCoeffs=63;
> +        nCoeffs = 64;
>       else
> -        nCoeffs= s->intra_scantable.raster_end[ s->block_last_index[n] ];
> +        nCoeffs = s->intra_scantable.raster_end[s->block_last_index[n]] + 1;
>   
> -    for(i=1; i<=nCoeffs; i++) {
> -        level = block[i];
> -        if (level) {
> -            if (level < 0) {
> -                level = level * qmul - qadd;
> -            } else {
> -                level = level * qmul + qadd;
> -            }
> -            block[i] = level;
> -        }
> -    }
> +    s->h263dsp.h263_dct_unquantize_intra(block, nCoeffs, qmul, qadd);

Looking further into this, you're adding a function pointer call in a 
function that's already called from a function pointer. And both x86 and 
arm have asm optimized versions of this entire method, which includes 
all the setup before the loop.

Can't you do the same for riscv? Is there anything preventing you from 
accessing fields at specific offsets within MpegEncContext?

>   }
>   
>   static void dct_unquantize_h263_inter_c(MpegEncContext *s,
>                                     int16_t *block, int n, int qscale)
>   {
> -    int i, level, qmul, qadd;
> +    int qmul = qscale << 1;
> +    int qadd = (qscale - 1) | 1;
>       int nCoeffs;
>   
>       av_assert2(s->block_last_index[n]>=0);
>   
> -    qadd = (qscale - 1) | 1;
> -    qmul = qscale << 1;
> -
> -    nCoeffs= s->inter_scantable.raster_end[ s->block_last_index[n] ];
> -
> -    for(i=0; i<=nCoeffs; i++) {
> -        level = block[i];
> -        if (level) {
> -            if (level < 0) {
> -                level = level * qmul - qadd;
> -            } else {
> -                level = level * qmul + qadd;
> -            }
> -            block[i] = level;
> -        }
> -    }
> +    nCoeffs = s->inter_scantable.raster_end[s->block_last_index[n]] + 1;
> +    s->h263dsp.h263_dct_unquantize_inter(block, nCoeffs, qmul, qadd);
>   }
>   
>   
> @@ -275,6 +250,9 @@ static void gray8(uint8_t *dst, const uint8_t *src, ptrdiff_t linesize, int h)
>   static av_cold int dct_init(MpegEncContext *s)
>   {
>       ff_blockdsp_init(&s->bdsp);
> +#if CONFIG_H263DSP
> +    ff_h263dsp_init(&s->h263dsp);
> +#endif
>       ff_hpeldsp_init(&s->hdsp, s->avctx->flags);
>       ff_videodsp_init(&s->vdsp, s->avctx->bits_per_raw_sample);
>