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

Ramiro Polla ramiro.polla
Sun Jan 24 06:14:47 CET 2010


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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cpu_reg.diff
Type: text/x-diff
Size: 3116 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100124/b014caca/attachment.diff>



More information about the ffmpeg-devel mailing list