Hi,

On Sat, May 28, 2011 at 3:29 AM, Kostya <[email protected]> wrote:
> On Fri, May 27, 2011 at 07:01:47PM -0400, Ronald S. Bultje wrote:
>> ---
>>  libswscale/swscale_internal.h     |    2 +
>>  libswscale/utils.c                |    1 +
>>  libswscale/x86/swscale_template.c |  141 
>> ++++++++++++++++--------------------
>>  3 files changed, 66 insertions(+), 78 deletions(-)
>>
>> diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
>> index b7183d3..c17d550 100644
>> --- a/libswscale/swscale_internal.h
>> +++ b/libswscale/swscale_internal.h
>> @@ -194,6 +194,7 @@ typedef struct SwsContext {
>>  #define Y_TEMP                "11*8+4*4*256*2+40"
>>  #define ALP_MMX_FILTER_OFFSET "11*8+4*4*256*2+48"
>>  #define UV_OFF                "11*8+4*4*256*3+48"
>> +#define UV_OFFx2              "11*8+4*4*256*3+56"
>>
>>      DECLARE_ALIGNED(8, uint64_t, redDither);
>>      DECLARE_ALIGNED(8, uint64_t, greenDither);
>> @@ -217,6 +218,7 @@ typedef struct SwsContext {
>>      DECLARE_ALIGNED(8, uint64_t, y_temp);
>>      int32_t  alpMmxFilter[4*MAX_FILTER_SIZE];
>>      DECLARE_ALIGNED(8, ptrdiff_t, uv_off); ///< offset (in pixels) between 
>> u and v planes
>> +    DECLARE_ALIGNED(8, ptrdiff_t, uv_offx2); ///< offset (in bytes) between 
>> u and v planes
>
> Let alone misleading naming (I'd expect UV_OFFx2 = UV_OFF*2) what are you
> trying to achieve here with slightly different define which replaces some
> parameter?

The problem in the asm code is that it doesn't mark the stack (ebp)
and some regs (e.g. ebx) as clobbered, yet uses it inside (it simply
pushes/pops it). The result is that you can't use stack offsets or
things that depend on any of these. Thus, the current code broke. Yet
another reason this inline asm is quite ugly. By using the hardcoded
struct offsets, we can reuse a struct register (c) plus hardcoded
offset. That is the crash-fix (changing "#uv_off" to "UV_OFF"("#c")

However, then we run into the problem that this asm macro needs uv_off
in bytes (i.e. << 1) instead of pixels (since here we use int16_t
intermediates, and if it's not a register I can't use an x, 2
addressing trick), so I added the <<= 1 into the struct (since I can't
use stack variables). Other functions use the non<<=1-one, so I'd like
to keep that one also. Hence this patch also adds uv_offx2 into
swsContext.

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to