[Ffmpeg-devel] [patch] Snow - add_yblock x86 asm

Guillaume POIRIER poirierg
Thu Nov 10 11:34:01 CET 2005


Hi,

On 4/9/05, Michael Niedermayer <michaelni at gmx.at> wrote:
> Hi
>
> On Saturday 09 April 2005 16:05, Yartrebo wrote:
> > On Sat, 2005-04-09 at 12:32 +0200, Michael Niedermayer wrote:
> > > Hi
> > >
> > > On Saturday 09 April 2005 07:51, Yartrebo wrote:
> > > > Sorry, I attached a similarly named but totally different patch. I've
> > > > now attached the right patch.
> > >
> > > [...]
> > >
> > > > --- libavcodec/snow.c   5 Apr 2005 17:59:05 -0000       1.47
> > > > +++ libavcodec/snow.c   9 Apr 2005 05:03:19 -0000
> > >
> > > [...]
> > >
> > > > +#include "i386/mmx.h"
> > > > +#include "i386/snow_mmx_sse2.h"
> > >
> > > stuff in liavcodec/ should not include stuff from libavcodec/i386 or
> > > other processor specific directories
> >
> > okay, if you like it that way. Could you tell me how to do the run-time
> > code selection and how to place it in dsputil?
>
> just look at libavcodec/{,i386/}dsputil*.{c,h}, its very simple
> theres a DSPContext with function pointers, which get set to the specific
> implementation like plain c or mmx
> the actual implementations should also be in their own files as dsputil*.c is
> already quite big (snow_dsp.c and snow_dsp_mmx.c might be possible names)
>
>
> >
> > > > +#ifdef HAVE_MMX
> > > > +#define LOG2_OBMC_MAX 8
> > > > +#else
> > > >  #define LOG2_OBMC_MAX 6
> > > > +#endif
> > >
> > > same as above, HAVE_MMX/HAVE_ALTIVEC should be avoided in files in
> > > libavcodec/, there are exceptions where its ok, but here i dont see why
> > > the prozessor specific code shouldnt be entirely separate, please also
> > > keep in mind that mmx/sse availability is decided at runtime and not
> > > compiletime, and that the optimized code should be called through dsputil
> > > and why not simply set LOG2_OBMC_MAX to 8 for c too?
> >
> > LOG2_OBMC_MAX is kept to 6 for the C because you wanted the minimum bits
> > possible for the general implementation.
>
> i just wanted to keep the constants fit into 6bits not necessary keep the
> actual table unchanged, i have no problem with multiplying them with 4 or
> 1024 or whatever as long as it doesnt change the bitstream / we can reverse
> it if its slower on some cpu, without breaking compatibility
>
>
> [...]
> > > > +#define add_yblock_bw_8_obmc_16_mmx \
> > > > +        pxor_r2r(mm7, mm7);\
> > > > +        pcmpeqd_r2r(mm4, mm4);\
> > > > +        pslld_i2r(31, mm4);\
> > >
> > > please dont mix mmx.h with asm() style
> > >
> > > > +             "movq (%%esi), %%xmm0;       \n\t"\
> > > > +             "movl 12(%%eax), %%edx;      \n\t"\
> > >
> > > i dont think this will work on x86-64
> >
> > Unfortunately, none of my pointer code will work on x86-64. To be
> > honest, I thought that mplayer assumed that int == * == int32_t == 4 in
> > a lot of places and that mplayer would be forever 32 bit.
> >
> > Would you have any suggestions as to how to make code run in both 32 bit
> > and 64 bit modes without much or any loss of performance?
>
> please see the x86-64 patch, which should explain the problem better
> http://www1.mplayerhq.hu/cgi-bin/cvsweb.cgi/ffmpeg/libavcodec/i386/dsputil_mmx.c.diff?r1=1.87&r2=1.88&cvsroot=FFMpeg
>
> [...]
> --
> Michael
>
> "nothing is evil in the beginning. Even Sauron was not so." -- Elrond

Yartrebo, did you make some progress on those patches lately? If so,
could you maybe post the latest version of them so that they get a
chance to be examined, fixed, and maybe committed?

Guillaume
--
Reading doesn't hurt, really!
  -- Dominik 'Rathann' Mierzejewski





More information about the ffmpeg-devel mailing list