[FFmpeg-cvslog] r29010 - in trunk/libswscale: yuv2rgb.c yuv2rgb_template.c

[David Conrad]

On Mar 21, 2009, at 6:17 AM, Reimar D?ffinger wrote:

> On Fri, Mar 20, 2009 at 08:42:55PM +0100, C?dric Schieli wrote:
>> 2009/3/20 Reimar D?ffinger <Reimar.Doeffinger at gmx.de>:
>>> On Fri, Mar 20, 2009 at 03:14:28PM +0100, sdrik wrote:
>>>> Modified: trunk/libswscale/yuv2rgb_template.c
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> ===================================================================
>>>> --- trunk/libswscale/yuv2rgb_template.c       Fri Mar 20 15:01:51  
>>>> 2009        (r29009)
>>>> +++ trunk/libswscale/yuv2rgb_template.c       Fri Mar 20 15:14:28  
>>>> 2009        (r29010)
>>>> @@ -171,6 +171,14 @@
>>>>      __asm__ volatile (EMMS); \
>>>>      return srcSliceH; \
>>>>
>>>> +#define YUV2RGB_OPERANDS_ALPHA \
>>>> +        : "+r" (index), "+r" (image) \
>>>> +        : "r" (pu - index), "r" (pv - index), "r"(&c- 
>>>> >redDither), "r" (py - 2*index), "r" (pa - 2*index) \
>>>> +        ); \
>>>> +    } \
>>>> +    __asm__ volatile (EMMS); \
>>>> +    return srcSliceH; \
>>>> +
>>>>  static inline int RENAME(yuv420_rgb16)(SwsContext *c, uint8_t*  
>>>> src[], int srcStride[], int srcSliceY,
>>>>                                         int srcSliceH, uint8_t*  
>>>> dst[], int dstStride[]){
>>>>      int y, h_size;
>>>> @@ -456,3 +464,19 @@ static inline int RENAME(yuv420_rgb32)(S
>>>>      YUV2RGB_ENDLOOP(4)
>>>>      YUV2RGB_OPERANDS
>>>>  }
>>>> +
>>>> +static inline int RENAME(yuva420_rgb32)(SwsContext *c, uint8_t*  
>>>> src[], int srcStride[], int srcSliceY,
>>>> +                                        int srcSliceH, uint8_t*  
>>>> dst[], int dstStride[]){
>>>> +    int y, h_size;
>>>> +
>>>> +    YUV2RGB_LOOP(4)
>>>> +
>>>> +        uint8_t *pa = src[3] + y*srcStride[3];
>>>> +        YUV2RGB_INIT
>>>> +        YUV2RGB
>>>> +        "movq     (%6, %0, 2), %%mm3;"            /* Load 8 A A7  
>>>> A6 A5 A4 A3 A2 A1 A0 */
>>>> +        RGB_PLANAR2PACKED32
>>>> +
>>>> +    YUV2RGB_ENDLOOP(4)
>>>> +    YUV2RGB_OPERANDS_ALPHA
>>>> +}
>>>
>>> Do _not_ commit assembler code without checking compilation with - 
>>> fPIC
>>> and --enable-debug.
>>> This uses 7 registers and can only be used when the appropriate  
>>> define
>>> is set.
>>
>> That's why HAVE_7REGS is checked at the callsite :
>
> Well, then the function is not called, but it is still compiled, so  
> that
> doesn't help a bit.
>
>> It build for me with --enable-debug --extra-cflags=-fPIC on x86_32
>
> Forgot --enable-gpl maybe?
> Either way it is certain it broke the 32 bit OSX build:
> http://fate.multimedia.cx/index.php?build_record=66730

gcc on OS X tends to much buggier when dealing with inline asm + PIC  
(it can barf with named register constraints, and >= 4.2 is completely  
broken) so it's quite possible (and has happened, see cavs asm) for  
code that works under linux to break for no apparent reason on OS X.

But yes, the proper usage of HAVE_7REGS is to #if out the actual  
inline asm that needs it and should fix this.