[FFmpeg-devel] [PATCH] Move H.264 DSP functions from dsputil.c to h264dsp.c
Aurelien Jacobs
aurel
Thu Jul 26 19:48:06 CEST 2007
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
More information about the ffmpeg-devel
mailing list