On Sat, May 28, 2011 at 06:47:47AM -0400, Ronald S. Bultje wrote:
> 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.

fine with me then (if tested under valgrind for both x86 and x86_64)
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to