diff mbox series

[FFmpeg-devel] vc1dsp: Change remaining stride parameters to ptrdiff_t

Message ID 20220329124448.2593145-1-martin@martin.st
State Accepted
Headers show
Series [FFmpeg-devel] vc1dsp: Change remaining stride parameters to ptrdiff_t | expand

Checks

Context Check Description
andriy/make_aarch64_jetson success Make finished
andriy/make_fate_aarch64_jetson success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Martin Storsjö March 29, 2022, 12:44 p.m. UTC 
The existing x86 assembly for loop filters uses the stride as a
full register without clearing/sign extending the upper half
of the registers on x86_64.

This avoids crashes if the caller would have passed nonzero bits
in the previously undefined upper 32 bits of the parameters.

Signed-off-by: Martin Storsjö <martin@martin.st>
---
 libavcodec/vc1dsp.c          | 20 ++++++++++----------
 libavcodec/vc1dsp.h          | 16 ++++++++--------
 libavcodec/x86/vc1dsp_init.c | 16 ++++++++--------
 3 files changed, 26 insertions(+), 26 deletions(-)

Comments

Ben Avison March 29, 2022, 7:02 p.m. UTC | #1 
On 29/03/2022 13:44, Martin Storsjö wrote:
> The existing x86 assembly for loop filters uses the stride as a
> full register without clearing/sign extending the upper half
> of the registers on x86_64.
> 
> This avoids crashes if the caller would have passed nonzero bits
> in the previously undefined upper 32 bits of the parameters.

Looks perfectly reasonable to me. I'll hold off making matching changes 
to my upcoming Arm versions until this gets merged. (My 64-bit Arm 
versions followed the old prototype and sign-extended the strides from 
32 to 64 bits, but that won't be necessary any more.)

Ben
Martin Storsjö March 30, 2022, 12:05 p.m. UTC | #2 
On Tue, 29 Mar 2022, Ben Avison wrote:

> On 29/03/2022 13:44, Martin Storsjö wrote:
>> The existing x86 assembly for loop filters uses the stride as a
>> full register without clearing/sign extending the upper half
>> of the registers on x86_64.
>> 
>> This avoids crashes if the caller would have passed nonzero bits
>> in the previously undefined upper 32 bits of the parameters.
>
> Looks perfectly reasonable to me. I'll hold off making matching changes 
> to my upcoming Arm versions until this gets merged. (My 64-bit Arm 
> versions followed the old prototype and sign-extended the strides from 
> 32 to 64 bits, but that won't be necessary any more.)

Acked by Lynne on irc too - so I pushed it now.

// Martin
diff mbox series

Patch

diff --git a/libavcodec/vc1dsp.c b/libavcodec/vc1dsp.c
index a29b91bf3d..c567bc43e8 100644
--- a/libavcodec/vc1dsp.c
+++ b/libavcodec/vc1dsp.c
@@ -36,7 +36,7 @@ 
 #include "startcode.h"
 
 /* Apply overlap transform to horizontal edge */
-static void vc1_v_overlap_c(uint8_t *src, int stride)
+static void vc1_v_overlap_c(uint8_t *src, ptrdiff_t stride)
 {
     int i;
     int a, b, c, d;
@@ -60,7 +60,7 @@  static void vc1_v_overlap_c(uint8_t *src, int stride)
 }
 
 /* Apply overlap transform to vertical edge */
-static void vc1_h_overlap_c(uint8_t *src, int stride)
+static void vc1_h_overlap_c(uint8_t *src, ptrdiff_t stride)
 {
     int i;
     int a, b, c, d;
@@ -146,7 +146,7 @@  static void vc1_h_s_overlap_c(int16_t *left, int16_t *right, int left_stride, in
  * @return whether other 3 pairs should be filtered or not
  * @see 8.6
  */
-static av_always_inline int vc1_filter_line(uint8_t *src, int stride, int pq)
+static av_always_inline int vc1_filter_line(uint8_t *src, ptrdiff_t stride, int pq)
 {
     int a0 = (2 * (src[-2 * stride] - src[1 * stride]) -
               5 * (src[-1 * stride] - src[0 * stride]) + 4) >> 3;
@@ -195,7 +195,7 @@  static av_always_inline int vc1_filter_line(uint8_t *src, int stride, int pq)
  * @param pq block quantizer
  * @see 8.6
  */
-static inline void vc1_loop_filter(uint8_t *src, int step, int stride,
+static inline void vc1_loop_filter(uint8_t *src, int step, ptrdiff_t stride,
                                    int len, int pq)
 {
     int i;
@@ -212,32 +212,32 @@  static inline void vc1_loop_filter(uint8_t *src, int step, int stride,
     }
 }
 
-static void vc1_v_loop_filter4_c(uint8_t *src, int stride, int pq)
+static void vc1_v_loop_filter4_c(uint8_t *src, ptrdiff_t stride, int pq)
 {
     vc1_loop_filter(src, 1, stride, 4, pq);
 }
 
-static void vc1_h_loop_filter4_c(uint8_t *src, int stride, int pq)
+static void vc1_h_loop_filter4_c(uint8_t *src, ptrdiff_t stride, int pq)
 {
     vc1_loop_filter(src, stride, 1, 4, pq);
 }
 
-static void vc1_v_loop_filter8_c(uint8_t *src, int stride, int pq)
+static void vc1_v_loop_filter8_c(uint8_t *src, ptrdiff_t stride, int pq)
 {
     vc1_loop_filter(src, 1, stride, 8, pq);
 }
 
-static void vc1_h_loop_filter8_c(uint8_t *src, int stride, int pq)
+static void vc1_h_loop_filter8_c(uint8_t *src, ptrdiff_t stride, int pq)
 {
     vc1_loop_filter(src, stride, 1, 8, pq);
 }
 
-static void vc1_v_loop_filter16_c(uint8_t *src, int stride, int pq)
+static void vc1_v_loop_filter16_c(uint8_t *src, ptrdiff_t stride, int pq)
 {
     vc1_loop_filter(src, 1, stride, 16, pq);
 }
 
-static void vc1_h_loop_filter16_c(uint8_t *src, int stride, int pq)
+static void vc1_h_loop_filter16_c(uint8_t *src, ptrdiff_t stride, int pq)
 {
     vc1_loop_filter(src, stride, 1, 16, pq);
 }
diff --git a/libavcodec/vc1dsp.h b/libavcodec/vc1dsp.h
index c6443acb20..34f3c89d85 100644
--- a/libavcodec/vc1dsp.h
+++ b/libavcodec/vc1dsp.h
@@ -42,16 +42,16 @@  typedef struct VC1DSPContext {
     void (*vc1_inv_trans_8x4_dc)(uint8_t *dest, ptrdiff_t stride, int16_t *block);
     void (*vc1_inv_trans_4x8_dc)(uint8_t *dest, ptrdiff_t stride, int16_t *block);
     void (*vc1_inv_trans_4x4_dc)(uint8_t *dest, ptrdiff_t stride, int16_t *block);
-    void (*vc1_v_overlap)(uint8_t *src, int stride);
-    void (*vc1_h_overlap)(uint8_t *src, int stride);
+    void (*vc1_v_overlap)(uint8_t *src, ptrdiff_t stride);
+    void (*vc1_h_overlap)(uint8_t *src, ptrdiff_t stride);
     void (*vc1_v_s_overlap)(int16_t *top,  int16_t *bottom);
     void (*vc1_h_s_overlap)(int16_t *left, int16_t *right, int left_stride, int right_stride, int flags);
-    void (*vc1_v_loop_filter4)(uint8_t *src, int stride, int pq);
-    void (*vc1_h_loop_filter4)(uint8_t *src, int stride, int pq);
-    void (*vc1_v_loop_filter8)(uint8_t *src, int stride, int pq);
-    void (*vc1_h_loop_filter8)(uint8_t *src, int stride, int pq);
-    void (*vc1_v_loop_filter16)(uint8_t *src, int stride, int pq);
-    void (*vc1_h_loop_filter16)(uint8_t *src, int stride, int pq);
+    void (*vc1_v_loop_filter4)(uint8_t *src, ptrdiff_t stride, int pq);
+    void (*vc1_h_loop_filter4)(uint8_t *src, ptrdiff_t stride, int pq);
+    void (*vc1_v_loop_filter8)(uint8_t *src, ptrdiff_t stride, int pq);
+    void (*vc1_h_loop_filter8)(uint8_t *src, ptrdiff_t stride, int pq);
+    void (*vc1_v_loop_filter16)(uint8_t *src, ptrdiff_t stride, int pq);
+    void (*vc1_h_loop_filter16)(uint8_t *src, ptrdiff_t stride, int pq);
 
     /* put 8x8 block with bicubic interpolation and quarterpel precision
      * last argument is actually round value instead of height
diff --git a/libavcodec/x86/vc1dsp_init.c b/libavcodec/x86/vc1dsp_init.c
index 8e0c284cc0..2fbf0b3a74 100644
--- a/libavcodec/x86/vc1dsp_init.c
+++ b/libavcodec/x86/vc1dsp_init.c
@@ -34,18 +34,18 @@ 
 #include "config.h"
 
 #define LOOP_FILTER(EXT) \
-void ff_vc1_v_loop_filter4_ ## EXT(uint8_t *src, int stride, int pq); \
-void ff_vc1_h_loop_filter4_ ## EXT(uint8_t *src, int stride, int pq); \
-void ff_vc1_v_loop_filter8_ ## EXT(uint8_t *src, int stride, int pq); \
-void ff_vc1_h_loop_filter8_ ## EXT(uint8_t *src, int stride, int pq); \
+void ff_vc1_v_loop_filter4_ ## EXT(uint8_t *src, ptrdiff_t stride, int pq); \
+void ff_vc1_h_loop_filter4_ ## EXT(uint8_t *src, ptrdiff_t stride, int pq); \
+void ff_vc1_v_loop_filter8_ ## EXT(uint8_t *src, ptrdiff_t stride, int pq); \
+void ff_vc1_h_loop_filter8_ ## EXT(uint8_t *src, ptrdiff_t stride, int pq); \
 \
-static void vc1_v_loop_filter16_ ## EXT(uint8_t *src, int stride, int pq) \
+static void vc1_v_loop_filter16_ ## EXT(uint8_t *src, ptrdiff_t stride, int pq) \
 { \
     ff_vc1_v_loop_filter8_ ## EXT(src,   stride, pq); \
     ff_vc1_v_loop_filter8_ ## EXT(src+8, stride, pq); \
 } \
 \
-static void vc1_h_loop_filter16_ ## EXT(uint8_t *src, int stride, int pq) \
+static void vc1_h_loop_filter16_ ## EXT(uint8_t *src, ptrdiff_t stride, int pq) \
 { \
     ff_vc1_h_loop_filter8_ ## EXT(src,          stride, pq); \
     ff_vc1_h_loop_filter8_ ## EXT(src+8*stride, stride, pq); \
@@ -56,9 +56,9 @@  LOOP_FILTER(mmxext)
 LOOP_FILTER(sse2)
 LOOP_FILTER(ssse3)
 
-void ff_vc1_h_loop_filter8_sse4(uint8_t *src, int stride, int pq);
+void ff_vc1_h_loop_filter8_sse4(uint8_t *src, ptrdiff_t stride, int pq);
 
-static void vc1_h_loop_filter16_sse4(uint8_t *src, int stride, int pq)
+static void vc1_h_loop_filter16_sse4(uint8_t *src, ptrdiff_t stride, int pq)
 {
     ff_vc1_h_loop_filter8_sse4(src,          stride, pq);
     ff_vc1_h_loop_filter8_sse4(src+8*stride, stride, pq);