On Thu, Feb 14, 2013 at 7:59 AM, Diego Biurrun <[email protected]> wrote:
> On Wed, Feb 13, 2013 at 05:53:36PM -0500, Daniel Kang wrote:
>>
>> --- a/libavcodec/x86/cavsdsp.c
>> +++ b/libavcodec/x86/cavsdsp.c
>> @@ -475,12 +481,18 @@ CAVS_MC(put_, 8, 3dnow)
>> CAVS_MC(put_, 16,3dnow)
>> CAVS_MC(avg_, 8, 3dnow)
>> CAVS_MC(avg_, 16,3dnow)
>> +#endif /* HAVE_AMD3DNOW_INLINE */
>>
>> static av_cold void ff_cavsdsp_init_3dnow(CAVSDSPContext *c,
>> AVCodecContext *avctx)
>> {
>> +#if HAVE_YASM
>> + c->put_cavs_qpel_pixels_tab[0][0] = ff_put_cavs_qpel16_mc00_mmxext;
>> + c->put_cavs_qpel_pixels_tab[1][0] = ff_put_cavs_qpel16_mc00_mmxext;
>> +#endif
>>
>> +#if HAVE_INLINE_ASM
>> #define dspfunc(PFX, IDX, NUM) \
>> - c->PFX ## _pixels_tab[IDX][ 0] = ff_ ## PFX ## NUM ## _mc00_mmxext; \
>> c->PFX ## _pixels_tab[IDX][ 2] = ff_ ## PFX ## NUM ## _mc20_3dnow; \
>> c->PFX ## _pixels_tab[IDX][ 4] = ff_ ## PFX ## NUM ## _mc01_3dnow; \
>
> mmxext functions in the 3dnow init function?
Yes this is correct. Does not contain any mmxext specific instructions.
>> --- a/libavcodec/x86/dsputil_mmx.c
>> +++ b/libavcodec/x86/dsputil_mmx.c
>> @@ -128,26 +136,45 @@ void ff_put_no_rnd_pixels8_y2_exact_mmxext(uint8_t
>> *block,
>> void ff_put_pixels8_mmxext(uint8_t *block, const uint8_t *pixels, ptrdiff_t
>> line_size, int h);
>> static void ff_put_pixels16_mmxext(uint8_t *block, const uint8_t *pixels,
>> - int line_size, int h)
>> + ptrdiff_t line_size, int h)
>
> Is there a reason not to do this separately, i.e. right away?
No.
>> --- a/libavcodec/x86/dsputil_rnd_template.c
>> +++ b/libavcodec/x86/dsputil_rnd_template.c
>> @@ -25,570 +25,28 @@
>>
>> //FIXME optimize
>> -static void DEF(put, pixels16_y2)(uint8_t *block, const uint8_t *pixels,
>> ptrdiff_t line_size, int h){
>> - DEF(put, pixels8_y2)(block , pixels , line_size, h);
>> - DEF(put, pixels8_y2)(block+8, pixels+8, line_size, h);
>> +static void DEF(ff_put, pixels16_y2)(uint8_t *block, const uint8_t *pixels,
>> ptrdiff_t line_size, int h){
>> + DEF(ff_put, pixels8_y2)(block , pixels , line_size, h);
>> + DEF(ff_put, pixels8_y2)(block+8, pixels+8, line_size, h);
>> }
>
> Is the FIXME comment still valid in some way?
Yes and no. There are mmxext versions of the same thing and they're
faster anyway.
> Please prettyprint those lines that you are changing anyway, I think
> I gave you an sed expression to do it automatically the last time.
> It should still work and/or be easy to adopt.
Done.
>> @@ -56,6 +107,44 @@ PUT_PIXELS8_X2
>>
>> +%macro PUT_PIXELS8_X2_MMX 0-1
>> +%if %0 == 1
>> +cglobal put%1_pixels8_x2, 4,4
>> +%else
>> +cglobal put_pixels8_x2, 4,4
>> +%endif
>
> IIRC you don't need the %if, but you can just pass an empty
> first parameter and it should do the right thing.
> .. more below ..
I tried this (and Ronald's suggestion) and I get the error:
libavcodec/x86/hpeldsp.asm:142: error: (cglobal_internal:8) `%ifndef'
expects macro identifiers
I suspect this has to do with putting the %1 in the middle of the
string. Suggestions appreciated.
>> + lea r1, [r1+r2*2]
>> + lea r0, [r0+r2*2]
>> + sub r3d, 4
>> + jne .loop
>> + RET
>
> Weird placement of .loop; I suggest aligning it with the rest.
> Probably it is handled inconsistently throughout...
All my code I've written has the loop in that placement. It's very
possible it's inconsistent across files.
>> @@ -453,6 +753,201 @@ AVG_PIXELS8_XY2
>>
>> +%macro AVG_PIXELS8_XY2_MMX 0-1
>
> Some macros have comments with the C functions they implement, some
> don't. Please add the comments everywhere, I consider them helpful.
Done.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
