Re: [libav-devel] [PATCH 07/14] vc1: Change type of array stride parameters to ptrdiff_t
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
Re: [libav-devel] [PATCH 07/14] vc1: Change type of array stride parameters to ptrdiff_t
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 vc1: Change type of array stride parameters to ptrdiff_t ptrdiff_t is the correct type for array strides and similar. Also rename all such parameters to "stride" for consistency. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 07/14] vc1: Change type of array stride parameters to ptrdiff_t
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(-) diff --git a/libavcodec/x86/vc1dsp.asm b/libavcodec/x86/vc1dsp.asm index adf08d7..9136ad9 100644 --- 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. // Martin ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 07/14] vc1: Change type of array stride parameters to ptrdiff_t
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(-)
diff --git a/libavcodec/arm/vc1dsp_init_neon.c
b/libavcodec/arm/vc1dsp_init_neon.c
index 08c07c4..944d184 100644
--- a/libavcodec/arm/vc1dsp_init_neon.c
+++ b/libavcodec/arm/vc1dsp_init_neon.c
@@ -35,7 +35,7 @@ void ff_vc1_inv_trans_8x4_dc_neon(uint8_t *dest, ptrdiff_t
stride, int16_t *bloc
void ff_vc1_inv_trans_4x4_dc_neon(uint8_t *dest, ptrdiff_t stride, int16_t
*block);
void ff_put_pixels8x8_neon(uint8_t *block, const uint8_t *pixels,
- ptrdiff_t line_size, int rnd);
+ ptrdiff_t stride, int rnd);
void ff_put_vc1_mspel_mc10_neon(uint8_t *dst, const uint8_t *src,
ptrdiff_t stride, int rnd);
diff --git a/libavcodec/vc1.c b/libavcodec/vc1.c
index 7a93e97..27e3071 100644
--- a/libavcodec/vc1.c
+++ b/libavcodec/vc1.c
@@ -64,8 +64,8 @@ enum Imode {
* @param[in] height Height of this buffer
* @param[in] stride of this buffer
*/
-static void decode_rowskip(uint8_t* plane, int width, int height, int stride,
- GetBitContext *gb)
+static void decode_rowskip(uint8_t* plane, int width, int height,
+ ptrdiff_t stride, GetBitContext *gb)
{
int x, y;
@@ -86,8 +86,8 @@ static void decode_rowskip(uint8_t* plane, int width, int
height, int stride,
* @param[in] stride of this buffer
* @todo FIXME: Optimize
*/
-static void decode_colskip(uint8_t* plane, int width, int height, int stride,
- GetBitContext *gb)
+static void decode_colskip(uint8_t* plane, int width, int height,
+ ptrdiff_t stride, GetBitContext *gb)
{
int x, y;
@@ -115,7 +115,8 @@ static int bitplane_decoding(uint8_t* data, int *raw_flag,
VC1Context *v)
int imode, x, y, code, offset;
uint8_t invert, *planep = data;
-int width, height, stride;
+int width, height;
+ptrdiff_t stride;
width = v->s.mb_width;
height = v->s.mb_height >> v->field_mode;
diff --git a/libavcodec/vc1_block.c b/libavcodec/vc1_block.c
index 0e1018c..eee32f2 100644
--- a/libavcodec/vc1_block.c
+++ b/libavcodec/vc1_block.c
@@ -82,8 +82,8 @@ static void vc1_put_signed_blocks_clamped(VC1Context *v)
{
MpegEncContext *s = >s;
int topleft_mb_pos, top_mb_pos;
-int stride_y, fieldtx = 0;
-int v_dist;
+int v_dist, fieldtx = 0;
+ptrdiff_t stride_y;
/* The put pixels loop is always one MB row behind the decoding loop,
* because we can only put pixels when overlap filtering is done, and
@@ -348,7 +348,8 @@ static inline void vc1_b_mc(VC1Context *v, int dmv_x[2],
int dmv_y[2],
static inline int vc1_i_pred_dc(MpegEncContext *s, int overlap, int pq, int n,
int16_t **dc_val_ptr, int *dir_ptr)
{
-int a, b, c, wrap, pred, scale;
+int a, b, c, pred, scale;
+ptrdiff_t wrap;
int16_t *dc_val;
static const uint16_t dcpred[32] = {
-1, 1024, 512, 341, 256, 205, 171, 146, 128,
@@ -414,7 +415,8 @@ static inline int ff_vc1_pred_dc(MpegEncContext *s, int
overlap, int pq, int n,
int a_avail, int c_avail,
int16_t **dc_val_ptr, int *dir_ptr)
{
-int a, b, c, wrap, pred;
+int a, b, c, pred;
+ptrdiff_t wrap;
int16_t *dc_val;
int mb_pos = s->mb_x + s->mb_y * s->mb_stride;
int q1, q2 = 0;
@@ -490,7 +492,8 @@ static inline int ff_vc1_pred_dc(MpegEncContext *s, int
overlap, int pq, int n,
static inline int vc1_coded_block_pred(MpegEncContext * s, int n,
uint8_t **coded_block_ptr)
{
-int xy, wrap, pred, a, b, c;
+int xy, pred, a, b, c;
+ptrdiff_t wrap;
xy = s->block_index[n];
wrap = s->b8_stride;
@@ -1176,7 +1179,7 @@ static int vc1_decode_intra_block(VC1Context *v, int16_t
block[64], int n,
*/
static int vc1_decode_p_block(VC1Context *v, int16_t block[64], int n,
int mquant, int ttmb, int first_block,
- uint8_t *dst, int linesize, int skip_block,
+ uint8_t *dst, ptrdiff_t linesize, int skip_block,
