[FFmpeg-devel] [PATCH] split mpegvideo.c

[Benoit Fouet]

Benoit Fouet wrote:
> Hi,
>
> Michael Niedermayer wrote:
>   
>> Hi
>>
>> On Thu, Jun 28, 2007 at 01:38:23PM +0200, Benoit Fouet wrote:
>>   
>>     
>>> Benoit Fouet wrote:
>>>     
>>>       
>>>> M?ns Rullg?rd wrote:
>>>>   
>>>>       
>>>>         
>>>>> Benoit Fouet <benoit.fouet at purplelabs.com> writes:
>>>>>   
>>>>>     
>>>>>         
>>>>>           
>>>>>> Hi,
>>>>>>
>>>>>> here is a first attempt to $subj
>>>>>>
>>>>>> what is done:
>>>>>>  - remove everything flagged under CONFIG_ENCODERS from mpegvideo.c
>>>>>> (apart from the DCT_common_init() part)
>>>>>>  - put those parts in mpegvideo_enc.c
>>>>>>     
>>>>>>       
>>>>>>           
>>>>>>             
>>>>> Does this in any way affect performance?  I can imagine calls from
>>>>> encoding functions to common functions no longer being inlined due to
>>>>> the split.  Then again, there might not be any relevant such calls.
>>>>>
>>>>>   
>>>>>     
>>>>>         
>>>>>           
>>>> i didn't check performances yet. but this is a problem we might have, i
>>>> guess...
>>>>
>>>>   
>>>>       
>>>>         
>>> anyway, what would be the proper way to do such a benchmark.
>>> As it is something i never did in ffmpeg, any pointer on how to do it
>>> would be very welcome !
>>>     
>>>       
>> nm anychangedobject.o
>>
>> and checking if new function names appear (simply cleaning the output of nm
>> with sed and diffing them works well), if so they are no longer inlined
>> (this will tell us what needs benchmarking)
>>
>>   
>>     
>
> ok, i did that.
> it seems the only differences are the following ones:
>  - copy_picture is now defined
>  - MPV_common_defaults too
>
> the two of them were implicitely inlined by the compiler actually.
> i think copy_picture could be moved to the common header file to make it
> inlined again, but this should not be necessary for
> MPV_commmon_defaults, which is only called at initialization of codec.
>
>   
>> a time ffmpeg -i A B test for mpeg encoding and decoding
>> some START/STOP_TIMER tests for functions which we think might have changed
>> inlining behavior (see nm and what you changed in your patch)
>>
>>   
>>     
>
> i've juste tried to move copy_picture to header as an inlined function,
> and it make it disappear from nm output, so there are no difference
> left, apart from the MPV_common_defaults one between the nm output
> before and after the patch.
>
>   

i have an updated patch to let copy_picture in header file and inline
it, but i guess i'd better not send it (as it is a minor fix and the new
patch has the same enormous size)

-- 
Ben
Purple Labs S.A.
www.purplelabs.com