On Thu, 29 Sep 2016, Diego Biurrun wrote:
On Thu, Sep 29, 2016 at 02:24:32PM +0300, Martin Storsjö wrote:
On Tue, 20 Sep 2016, Diego Biurrun wrote:
> This avoids SIMD-optimized functions having to sign-extend their
> stride argument manually to be able to do pointer arithmetic.
>
> Also rename all such parameters to "stride" for consistency.
> ---
> libavcodec/arm/vc1dsp_init_neon.c | 2 +-
> libavcodec/vc1.c | 11 ++++++-----
> libavcodec/vc1_block.c | 21 +++++++++++++--------
> libavcodec/vc1_loopfilter.c | 8 +++++---
> libavcodec/vc1_pred.c | 9 ++++++---
> libavcodec/vc1dsp.c | 26 +++++++++++++-------------
> libavcodec/vc1dsp.h | 16 ++++++++--------
> libavcodec/x86/vc1dsp.asm | 22 +++++++++++-----------
> libavcodec/x86/vc1dsp_init.c | 16 ++++++++--------
> libavcodec/x86/vc1dsp_mmx.c | 21 +++++++++++----------
> 10 files changed, 82 insertions(+), 70 deletions(-)
> --- a/libavcodec/x86/vc1dsp.asm
> +++ b/libavcodec/x86/vc1dsp.asm
> @@ -237,19 +237,19 @@ cglobal vc1_h_loop_filter_internal
> VC1_H_LOOP_FILTER 4, r4
> ret
>
> -; void ff_vc1_v_loop_filter4_mmxext(uint8_t *src, int stride, int pq)
> +; void ff_vc1_v_loop_filter4_mmxext(uint8_t *src, ptrdiff_t stride, int pq)
> cglobal vc1_v_loop_filter4, 3,5,0
> START_V_FILTER
> call vc1_v_loop_filter_internal
> RET
I don't see the corresponding asm simplification as the commit message
touts. I.e., this is probably a latent bug; fix that first with the proper
sign extensions before scrambling things by changing the signature.
I think I just used the wrong log message on this one. Changed locally
to
No, not really. There's three ways it can be:
1) The function actually doesn't use the stride parameter, and no change
is needed
2) The function does use it correctly (e.g. doing a sign extension of it
somewhere, or using it via register names like 'rNd' or so, making it
explicitly that it's a 32 bit parameter). In those cases, we should
most probably update the asm accordingly, i.e. remove the sign extension,
or use 'rN' instead of 'rNd'.
3) The function doesn't use it correctly right now, and we have a bug that
should be fixed before we change the type.
In this case, I'm pretty sure that the parameter isn't unused in all those
functions, that would be highly surprising.
On a quick view, it seems like this parameter is used within the
START_V/H_FILTER macros, as 'r1', which should probably be 'r1d' (or sign
extended into r1 if one can't use r1d at those places - my x86 asm is very
rusty).
In general, when changing the type from int to ptrdiff_t, there should be
a change in every single asm function where you change the signature.
Otherwise the parameter is either unused, or there's a bug. I think. (If
you've spent more time looking at it and can explain why this isn't the
case, then please do.)
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
