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

[Panagiotis Issaris]

Hi Aurelien,

Aurelien Jacobs schreef:
> 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 :-)

Thanks :)

I'd also created a patch moving out the H.264 QPEL functions which had a
much nicer impact, but unfortunately it is rather messy.


> 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.

Fixed. I've removed the comment too as it seems rather obvious by the
functionname that it is H264 specific and as I was the one who added it
in the first place I'd think nobody would disagree.


>> 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).

Yep, I'll look into that 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).

Unfortunately, this won't work, as the H.264 encoder isn't in Subversion
yet.


> Except those remarks, the patch looks fine to me.

Thanks!

 Makefile  |    2
 dsputil.c |  319 ---------------------------------------------
 h264dsp.c |  321 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 323 insertions(+), 319 deletions(-)

With friendly regards,
Takis
--
vCard: http://issaris.org/pi.vcf
PGP key: http://issaris.org/pi.key
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: pi-20070727T110116-ffmpeg-dsputil_to_h264dsp.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070727/157a73f6/attachment.txt>