Hi,
Panagiotis Issaris schreef:
> Hi Aurelien,
>
> Aurelien Jacobs schreef:
>> On Thu, 26 Jul 2007 18:43:08 +0200
>> Panagiotis Issaris <takis.issaris 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!
Ouch, I just noticed that with this patch I had accidentally
moved lots
of the H.264 decoder DSP code in a file with a different
header,
resulting in incorrect "Copyright by ..." lines.
Is it okay to just merge the "Copyright by ..."
lines? Or would it be
better to split the h264dsp.c file right away in a
h264dspenc.c and
h264dspdec.c as Aurelien suggested?
With friendly regards,
Takis
--
vCard: http://issaris.org/pi.vcf
PGP key: http://issaris.org/pi.key
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-develmplayerhq.hu
http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
|
