[FFmpeg-devel] [PATCH] Fix function parameters for rgb48 to YV12 functions.

Michael Niedermayer michaelni
Sun Jan 24 11:31:39 CET 2010


On Sun, Jan 24, 2010 at 03:14:47AM -0200, Ramiro Polla wrote:
> On Thu, Jan 14, 2010 at 1:11 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Thu, Jan 14, 2010 at 02:10:56PM +0000, M?ns Rullg?rd wrote:
> >> Ramiro Polla <ramiro.polla at gmail.com> writes:
> >> > 2010/1/14 M?ns Rullg?rd <mans at mansr.com>:
> >> >> Michael Niedermayer <michaelni at gmx.at> writes:
> >> >>> On Thu, Jan 14, 2010 at 04:25:01AM -0200, Ramiro Polla wrote:
> >> >>>> On Mon, Jan 11, 2010 at 12:51 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> >>>> > On Mon, Jan 11, 2010 at 05:21:19AM -0200, Ramiro Polla wrote:
> >> >>> [...]
> >> >>>> >> ?swscale.c | ? ?6 +++---
> >> >>>> >> ?1 file changed, 3 insertions(+), 3 deletions(-)
> >> >>>> >> 104e7c17caec26638ce07bb04dc15eaa0d9133fb ?0003-int-long-for-width-parameter-in-rgb48-to-YV12-fun.patch
> >> >>>> >> From da0f8f381054f23a32251981354ec56fb1232a56 Mon Sep 17 00:00:00 2001
> >> >>>> >> From: Ramiro Polla <ramiro.polla at gmail.com>
> >> >>>> >> Date: Mon, 11 Jan 2010 05:04:09 -0200
> >> >>>> >> Subject: [PATCH] int -> long for width parameter in rgb48 to YV12 functions.
> >> >>>> >
> >> >>>> > dont we have a proper type for that?
> >> >>>>
> >> >>>> You mean x86_reg? That wouldn't be nice in a function declaration.
> >> >>>>
> >> >>>> What I have in mind now is changing them all to int, and cast to
> >> >>>> x86_reg (or use local x86_reg variables) only inside x86 or mmx
> >> >>>> ifdefs. What do you think about this approach?
> >> >>>
> >> >>> whats the problem with x86_reg in a declaration?
> >> >>
> >> >> Using x86_reg in non-x86-specific code is ugly. ?In fact, it shouldn't
> >> >> even compile.
> >> >
> >> > in libavutil:x86_cpu.h
> >> > #if ARCH_X86_64
> >> > [...]
> >> > typedef int64_t x86_reg;
> >> > [...]
> >> > #elif ARCH_X86_32
> >> > [...]
> >> > typedef int32_t x86_reg;
> >> > [...]
> >> > #else
> >> > typedef int x86_reg;
> >> > #endif
> >> >
> >> > So it does compile for now, but I'd like to remove that #else as well.
> >>
> >> IMO x86_cpu.h should not be included outside x86-specific code.
> >>
> >> > What do you think about that array_index type michael mentioned?
> >>
> >> What is special about array indexes? ?I thought the problem was
> >> ensuring the C type matches the operand type used in inline asm.
> >
> > 64bit array indexes will likely be faster on x86_64 in C code
> > (this of course needs a test, a codec like h264 seems like the most
> > ?important place to try it. If its not faster than we can drop the
> > ?array_index idea)
> 
> So basically we would still have some sort of type that is the width
> of the cpu's register and use it in the declaration? Attached patch is
> just an example (it's a hack) that uses x86_reg as cpu_reg so the code
> isn't x86-centric.
> 
> I don't really understand how that will help here. The width is int,
> so at some point it will have to be changed to cpu_reg, be it before
> calling, or in the function prologue. From the asm it looks like it
> just shuffles one movslq from caller to callee.

the gain happens when you change the variables used to calculate the index
also to it. You could also try to make the index unsigned but make sure it
cant be negative if you try this


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100124/2f1b9569/attachment.pgp>



More information about the ffmpeg-devel mailing list