[09/14] svq3: Change type of array stride parameters to ptrdiff_t

Message ID 1474396595-14910-9-git-send-email-diego@biurrun.de
State New
Headers show

Commit Message

Diego Biurrun Sept. 20, 2016, 6:36 p.m.
ptrdiff_t is the correct type for array strides and similar.
---
 libavcodec/svq3.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Mark Thompson Sept. 20, 2016, 8:22 p.m. | #1
On 20/09/16 19:36, Diego Biurrun wrote:
> ptrdiff_t is the correct type for array strides and similar.
> ---
>  libavcodec/svq3.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
> index aa85e7c..bd83731 100644
> --- a/libavcodec/svq3.c
> +++ b/libavcodec/svq3.c
> @@ -118,8 +118,8 @@ typedef struct SVQ3Context {
>      int mb_x, mb_y;
>      int mb_xy;
>      int mb_width, mb_height;
> -    int mb_stride, mb_num;
> -    int b_stride;
> +    int mb_num;
> +    ptrdiff_t mb_stride, b_stride;

mb_stride should stay an int.
Diego Biurrun Sept. 20, 2016, 9:07 p.m. | #2
On Tue, Sep 20, 2016 at 09:22:06PM +0100, Mark Thompson wrote:
> On 20/09/16 19:36, Diego Biurrun wrote:
> > ptrdiff_t is the correct type for array strides and similar.
> > --- a/libavcodec/svq3.c
> > +++ b/libavcodec/svq3.c
> > @@ -118,8 +118,8 @@ typedef struct SVQ3Context {
> >      int mb_x, mb_y;
> >      int mb_xy;
> >      int mb_width, mb_height;
> > -    int mb_stride, mb_num;
> > -    int b_stride;
> > +    int mb_num;
> > +    ptrdiff_t mb_stride, b_stride;
> 
> mb_stride should stay an int.

Why?

Diego
Mark Thompson Sept. 20, 2016, 9:21 p.m. | #3
On 20/09/16 22:07, Diego Biurrun wrote:
> On Tue, Sep 20, 2016 at 09:22:06PM +0100, Mark Thompson wrote:
>> On 20/09/16 19:36, Diego Biurrun wrote:
>>> ptrdiff_t is the correct type for array strides and similar.
>>> --- a/libavcodec/svq3.c
>>> +++ b/libavcodec/svq3.c
>>> @@ -118,8 +118,8 @@ typedef struct SVQ3Context {
>>>      int mb_x, mb_y;
>>>      int mb_xy;
>>>      int mb_width, mb_height;
>>> -    int mb_stride, mb_num;
>>> -    int b_stride;
>>> +    int mb_num;
>>> +    ptrdiff_t mb_stride, b_stride;
>>
>> mb_stride should stay an int.
> 
> Why?

It's used as:

(a) an offset against the current macroblock for prediction modes.

(b) a multiplier for y offsets to macroblock lines.

(c) a size for macroblock lines.

All of the other things there are ints (well, last one could be size_t), and have nothing to do with pointers.

- Mark
Diego Biurrun Sept. 22, 2016, 5:24 p.m. | #4
On Tue, Sep 20, 2016 at 10:21:40PM +0100, Mark Thompson wrote:
> On 20/09/16 22:07, Diego Biurrun wrote:
> > On Tue, Sep 20, 2016 at 09:22:06PM +0100, Mark Thompson wrote:
> >> On 20/09/16 19:36, Diego Biurrun wrote:
> >>> ptrdiff_t is the correct type for array strides and similar.
> >>> --- a/libavcodec/svq3.c
> >>> +++ b/libavcodec/svq3.c
> >>> @@ -118,8 +118,8 @@ typedef struct SVQ3Context {
> >>>      int mb_x, mb_y;
> >>>      int mb_xy;
> >>>      int mb_width, mb_height;
> >>> -    int mb_stride, mb_num;
> >>> -    int b_stride;
> >>> +    int mb_num;
> >>> +    ptrdiff_t mb_stride, b_stride;
> >>
> >> mb_stride should stay an int.
> > 
> > Why?
> 
> It's used as:
> 
> (a) an offset against the current macroblock for prediction modes.
> 
> (b) a multiplier for y offsets to macroblock lines.
> 
> (c) a size for macroblock lines.
> 
> All of the other things there are ints (well, last one could be size_t), and have nothing to do with pointers.

All of the above sounds stride-ish to me and we seem to use ptrdiff_t
for such variables elsewhere.  Ame I wrong?

Diego
Luca Barbato Sept. 22, 2016, 7:09 p.m. | #5
On 22/09/16 19:24, Diego Biurrun wrote:
> On Tue, Sep 20, 2016 at 10:21:40PM +0100, Mark Thompson wrote:
>> On 20/09/16 22:07, Diego Biurrun wrote:
>>> On Tue, Sep 20, 2016 at 09:22:06PM +0100, Mark Thompson wrote:
>>>> On 20/09/16 19:36, Diego Biurrun wrote:
>>>>> ptrdiff_t is the correct type for array strides and similar.
>>>>> --- a/libavcodec/svq3.c
>>>>> +++ b/libavcodec/svq3.c
>>>>> @@ -118,8 +118,8 @@ typedef struct SVQ3Context {
>>>>>      int mb_x, mb_y;
>>>>>      int mb_xy;
>>>>>      int mb_width, mb_height;
>>>>> -    int mb_stride, mb_num;
>>>>> -    int b_stride;
>>>>> +    int mb_num;
>>>>> +    ptrdiff_t mb_stride, b_stride;
>>>>
>>>> mb_stride should stay an int.
>>>
>>> Why?
>>
>> It's used as:
>>
>> (a) an offset against the current macroblock for prediction modes.
>>
>> (b) a multiplier for y offsets to macroblock lines.
>>
>> (c) a size for macroblock lines.
>>
>> All of the other things there are ints (well, last one could be size_t), and have nothing to do with pointers.
> 
> All of the above sounds stride-ish to me and we seem to use ptrdiff_t
> for such variables elsewhere.  Ame I wrong?

Probably, I think Mark is the second person finding wrong using ptrdiff
there.

lu
Martin Storsjö Sept. 29, 2016, 11:25 a.m. | #6
On Tue, 20 Sep 2016, Diego Biurrun wrote:

> ptrdiff_t is the correct type for array strides and similar.
> ---
> libavcodec/svq3.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)

Ok

// Martin

Patch

diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
index aa85e7c..bd83731 100644
--- a/libavcodec/svq3.c
+++ b/libavcodec/svq3.c
@@ -118,8 +118,8 @@  typedef struct SVQ3Context {
     int mb_x, mb_y;
     int mb_xy;
     int mb_width, mb_height;
-    int mb_stride, mb_num;
-    int b_stride;
+    int mb_num;
+    ptrdiff_t mb_stride, b_stride;
 
     uint32_t *mb2br_xy;
 
@@ -252,7 +252,7 @@  static void svq3_luma_dc_dequant_idct_c(int16_t *output, int16_t *input, int qp)
 #undef stride
 
 static void svq3_add_idct_c(uint8_t *dst, int16_t *block,
-                            int stride, int qp, int dc)
+                            ptrdiff_t stride, int qp, int dc)
 {
     const int qmul = svq3_dequant_coeff[qp];
     int i;
@@ -428,8 +428,8 @@  static inline void svq3_mc_dir_part(SVQ3Context *s,
     uint8_t *src, *dest;
     int i, emu = 0;
     int blocksize = 2 - (width >> 3); // 16->0, 8->1, 4->2
-    int linesize   = s->cur_pic->f->linesize[0];
-    int uvlinesize = s->cur_pic->f->linesize[1];
+    ptrdiff_t linesize   = s->cur_pic->f->linesize[0];
+    ptrdiff_t uvlinesize = s->cur_pic->f->linesize[1];
 
     mx += x;
     my += y;
@@ -609,7 +609,7 @@  static inline int svq3_mc_dir(SVQ3Context *s, int size, int mode,
 
 static av_always_inline void hl_decode_mb_idct_luma(SVQ3Context *s,
                                                     int mb_type, const int *block_offset,
-                                                    int linesize, uint8_t *dest_y)
+                                                    ptrdiff_t linesize, uint8_t *dest_y)
 {
     int i;
     if (!IS_INTRA4x4(mb_type)) {
@@ -630,7 +630,7 @@  static av_always_inline int dctcoef_get(int16_t *mb, int index)
 static av_always_inline void hl_decode_mb_predict_luma(SVQ3Context *s,
                                                        int mb_type,
                                                        const int *block_offset,
-                                                       int linesize,
+                                                       ptrdiff_t linesize,
                                                        uint8_t *dest_y)
 {
     int i;
@@ -673,7 +673,7 @@  static void hl_decode_mb(SVQ3Context *s)
     const int mb_xy   = s->mb_xy;
     const int mb_type = s->cur_pic->mb_type[mb_xy];
     uint8_t *dest_y, *dest_cb, *dest_cr;
-    int linesize, uvlinesize;
+    ptrdiff_t linesize, uvlinesize;
     int i, j;
     const int *block_offset = &s->block_offset[0];
     const int block_h   = 16 >> 1;
@@ -1329,7 +1329,7 @@  static int get_buffer(AVCodecContext *avctx, SVQ3Frame *pic)
     SVQ3Context *s = avctx->priv_data;
     const int big_mb_num    = s->mb_stride * (s->mb_height + 1) + 1;
     const int mb_array_size = s->mb_stride * s->mb_height;
-    const int b4_stride     = s->mb_width * 4 + 1;
+    const ptrdiff_t b4_stride = s->mb_width * 4 + 1;
     const int b4_array_size = b4_stride * s->mb_height * 4;
     int ret;