On Sat, Jan 5, 2013 at 5:47 PM, Diego Biurrun <[email protected]> wrote:
> On Sat, Jan 05, 2013 at 12:48:58PM -0500, Daniel Kang wrote:
>> On Sat, Jan 5, 2013 at 12:29 PM, Diego Biurrun <[email protected]> wrote:
>> > On Sat, Jan 05, 2013 at 11:01:19AM -0600, Daniel Kang wrote:
>> >>
>> >> --- a/libavfilter/x86/yadif.c
>> >> +++ b/libavfilter/x86/yadif.c
>> >>  av_cold void ff_yadif_init_x86(YADIFContext *yadif)
>> >>  {
>> >>      int cpu_flags = av_get_cpu_flags();
>> >>
>> >> -#if HAVE_MMXEXT_INLINE
>> >> +#if HAVE_YASM
>> >>      if (cpu_flags & AV_CPU_FLAG_MMXEXT)
>> >> -        yadif->filter_line = yadif_filter_line_mmxext;
>> >> -#endif
>> >> -#if HAVE_SSE2_INLINE
>> >> +        yadif->filter_line = ff_yadif_filter_line_mmxext;
>> >>      if (cpu_flags & AV_CPU_FLAG_SSE2)
>> >> -        yadif->filter_line = yadif_filter_line_sse2;
>> >> -#endif
>> >> -#if HAVE_SSSE3_INLINE
>> >> +        yadif->filter_line = ff_yadif_filter_line_sse2;
>> >>      if (cpu_flags & AV_CPU_FLAG_SSSE3)
>> >> -        yadif->filter_line = yadif_filter_line_ssse3;
>> >> +        yadif->filter_line = ff_yadif_filter_line_ssse3;
>> >>  #endif
>> >
>> > These could likely use HAVE_EXTERNAL_MMXEXT, etc...
>>
>> Maybe I'm missing something?
>>
>> AVX@AVX-PC /cygdrive/c/Code/libav
>> $ git grep HAVE_EXTERNAL
>> <nothing>
>
> I confused the order, it's HAVE_MMXEXT_EXTERNAL, etc...

Fixed.

>> >> +%macro CHECK1 0
>> >> +%endmacro
>> >> +
>> >> +%macro CHECK2 0
>> >> +%endmacro
>> >
>> > These names are not terribly descriptive.
>>
>> They weren't in the original file. I have no idea what to call them.
>
> Maybe ask Loren?

Asked.

>> >> +%macro LOAD 2
>> >> +    movh      m%1, %2
>> >> +    punpcklbw m%1, m7
>> >> +%endmacro
>> >> +
>> >> +%macro ABSY 1-2
>> >> +%if cpuflag(ssse3)
>> >> +    pabsw %1, %1
>> >> +%else
>> >> +    ABS1_MMXEXT %1, %2
>> >> +%endif
>> >> +%endmacro
>> >
>> > Is this a candidate for simply extending ABS1_MMXEXT?
>>
>> No. The MMXEXT is there for a reason.
>
> I was being totally unclear, as I was referring to some changes I only
> have locally.  Please review the following patch:
>
> http://patches.libav.org/patch/25264/

That looks right.

>> >> +%if mmsize == 16
>> >> +    mova       m3, m2
>> >> +    psrldq     m3, 2
>> >> +%else
>> >> +    pshufw     m3, m2, 9
>> >> +%endif
>> >
>> > Didn't we have a macro for this?
>>
>> If we do, I can't find it.
>
> h264_intrapred.asm has some very similar code that likely should be
> extracted into a common macro.

In another patch? This patch is supposed to be a port.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to