[FFmpeg-devel] [PATCH] Move H.264 DSP functions from dsputil.c to h264dsp.c

[Aurelien Jacobs]

On Thu, 26 Jul 2007 18:43:08 +0200
Panagiotis Issaris <takis.issaris at uhasselt.be> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi,
> 
> The attached patch moves the H.264 DSP functions from dsputil.c to
> h264dsp.c. Regression tests succeed with this patch applied.
> 
>  Makefile  |    2
>  dsputil.c |  320 ---------------------------------------------
>  h264dsp.c |  321 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 325 insertions(+), 318 deletions(-)
> 
> 
> I've tested the impact on the current SVN revision (9802) using various
> scenarios.
> 
> First I checked how the patch altered a minimally configured ffmpeg's
> codesize:
> 
> ./configure --disable-decoders --disable-demuxers --disable-parsers
> - --disable-protocols --disable-muxers --disable-encoders --disable-vhook
> - --disable-network --disable-zlib --disable-mmx
> 
> Without this patch:
>  323522       0    4484  328006   50146 libavcodec/dsputil.o
>  592586     904    8920  602410   9312a ffmpeg
> - -rw-r--r-- 1 takis takis 888916 2007-07-26 17:37 libavcodec/dsputil.o
> - -rwxr-xr-x 1 takis takis 598076 2007-07-26 17:37 ffmpeg
>  4191 libavcodec/dsputil.c
>    81 libavcodec/h264dsp.c
> 
> 
> With this patch:
>    text    data     bss     dec     hex filename
>  297858       0    4484  302342   49d06 libavcodec/dsputil.o
>  566922     904    8920  576746   8ccea ffmpeg
> - -rwxr-xr-x 1 takis takis 571388 2007-07-26 17:34 ffmpeg
> - -rw-r--r-- 1 takis takis 807848 2007-07-26 17:34 libavcodec/dsputil.o
>  3877 libavcodec/dsputil.c
>   402 libavcodec/h264dsp.c
> 
> Save 25664 bytes for ffmpeg in text segment,
> 26688 bytes in ffmpeg executable size.
> dsputil.o shrinks 25664 bytes.

Nice result :-)

Few remarks:

> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index e1685fe..9b05f62 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> 
> [...]
> 
> @@ -2558,10 +2394,10 @@ void ff_put_vc1_mspel_mc00_c(uint8_t *dst, uint8_t *src, int stride, int rnd) {
>  }
>  #endif /* CONFIG_VC1_DECODER||CONFIG_WMV3_DECODER */
>  
> -#if defined(CONFIG_H264_ENCODER)
> +#if defined(CONFIG_H264_DECODER) || defined(CONFIG_H264_ENCODER)
>  /* H264 specific */
>  void ff_h264dsp_init(DSPContext* c, AVCodecContext *avctx);
> -#endif /* CONFIG_H264_ENCODER */
> +#endif /* CONFIG_H264_DECODER || CONFIG_H264_ENCODER */

Simply remove the #ifdef here. It does no good at all.

> diff --git a/libavcodec/h264dsp.c b/libavcodec/h264dsp.c
> index 4f18afa..6a5dcbb 100644
> --- a/libavcodec/h264dsp.c
> +++ b/libavcodec/h264dsp.c
> @@ -28,6 +28,8 @@
>  
>  #include "dsputil.h"
>  
> +#if defined(CONFIG_H264_ENCODER)
> +
>  extern const uint8_t ff_div6[52];
>  extern const uint8_t ff_rem6[52];
>  
> @@ -73,9 +75,328 @@ static void h264_dct_c(DCTELEM block[4][4])
>      H264_DCT_PART2(2);
>      H264_DCT_PART2(3);
>  }
> +#endif /* CONFIG_H264_ENCODER */

Maybe spliting this part of the code into a h264dspenc.c file would be
even better (but that can be done later).

>  void ff_h264dsp_init(DSPContext* c, AVCodecContext *avctx)
>  {
> +#if defined(CONFIG_H264_ENCODER)
>      c->h264_dct = h264_dct_c;
> +#endif

Here you could use if (ENABLE_H264_ENCODER).

Except those remarks, the patch looks fine to me.

Aurel