[FFmpeg-devel] Multimedia extensions run-time support check

[Alexander Strange]

On Jun 24, 2008, at 9:46 PM, Michael Niedermayer wrote:

> On Tue, Jun 24, 2008 at 07:24:52PM -0400, Alexander Strange wrote:
>>
>> On Jun 24, 2008, at 4:23 PM, Michael Niedermayer wrote:
>>
>>> On Tue, Jun 24, 2008 at 05:43:49PM +0200, Laurent Desnogues wrote:
>>>> Hello,
>>>>
>>>> (I know this is not a very interesting matter, but it's a good way
>>>> for me to start messing with FFmpeg code;  if noone cares, let
>>>> me know and I'll use some adhoc solution that fits my needs.)
>>>>
>>>> Currently to know what multimedia extensions are supported,
>>>> the code checks the value of mm_flags or the return value of
>>>> mm_support() (cf. dsputil.h).
>>>>
>>>> My understanding is that this is only used to setup things at
>>>> initialization time.
>>>>
>>>> However I find things are inconsistent.  For instance:
>>>>
>>>> - mm_support is always declared but only defined for MMX,
>>>> IWMMXT (Xscale) and PPC
>>>> - mm_flags is declared for ARMV4L but only defined for
>>>> IWMMXT
>>>>
>>>> This is not a big deal at the moment since it only breaks
>>>> the build of some test programs for some configs, such as
>>>> dct-test when either MMX is not defined or when building
>>>> for ARM without IWMMXT.
>>>
>>> dct-test.c was broken by
>>>   Author: benoit
>>>   Date: Tue Mar 25 15:34:27 2008
>>>   New Revision: 12580
>>>
>>>   Log:
>>>   Set mm_flags in dct-test to fix floating math.
>>>   Patch by Alexander Strange \astrange ithinksw com/
>>> and
>>>   Author: gpoirier
>>>   Date: Sun Mar 30 12:29:28 2008
>>>   New Revision: 12628
>>>
>>>   Log:
>>>   check mm_flags for each DCT and skips the ones that can't be run
>>>   patch by Alexander Strange %astrange A ithinksw P com%
>>>
>>> fft-test.c was broken by
>>>   Author: diego
>>>   Date: Tue Jun 24 11:00:58 2008
>>>   New Revision: 13940
>>>
>>>   Log:
>>>   Remove redundant mm_flags declaration, fixes the warning:
>>>   libavcodec.a(dsputil_ppc.o): warning: definition of `mm_flags'
>>> overriding common
>>>   fft-test.o: warning: common is here
>>>
>>>
>>>>
>>>> Currently I only see two solutions:
>>>>
>>>> 1. correctly protect declarations in dsputil.h depending
>>>>  on ifdef's
>>>> 2. always define fake mm_flags and mm_support that
>>>> are or return zero.
>>>
>>> I do not belive mm_flags was ever intended to be accessed from  
>>> outside
>>> the specific optimized code. it just happened by mistake as people  
>>> tried
>>> to "fix" gcc warnings without bothering to fully understand the code
>>>
>>> mm_support() should be a #define 0 when optims are off.
>>
>> Is this OK?
>
> no, just use a variable that does not conflict with mm_flags. like a  
> local
> var or a different name.
> Besides i do not think fft-test.c should be linked with dsputil*

Oops, there was a miscommunication.

patch 1 - rename mm_flags in dct-test
patch 2 - just remove mm_flags setting from fft-test

fft.c doesn't actually check mm_flags, so fft-test always ends up  
using the current asm fft anyway. But it uses the C imdct, since  
ff_imdct_calc is the C imdct instead of calling the asm function  
pointer.

Testing only asm vs. ref sounds more useful to me, but I didn't write  
it. I guess it's fallen behind the API; anyway, this keeps the current  
behavior.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2-ffttest-mmflags.diff
Type: text/x-diff
Size: 390 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080624/d4056ac6/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1-dcttest-mmflags.diff
Type: text/x-diff
Size: 951 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080624/d4056ac6/attachment-0001.diff>