[FFmpeg-devel] [PATCH] swscale/swscale_unscaled: Fix undefined NULL + 0

[FFmpeg-devel] [PATCH] swscale/swscale_unscaled: Fix undefined NULL + 0

[Andreas Rheinhardt]

Affected the fitsdec-gbrp16 FATE-test.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libswscale/swscale_unscaled.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
index 8838cc8b53..0b97377934 100644
--- a/libswscale/swscale_unscaled.c
+++ b/libswscale/swscale_unscaled.c
@@ -695,7 +695,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
             }
         }
         for (i = 0; i < 4; i++)
-            dst[i] += dstStride[i] >> 1;
+            dst[i] = FF_PTR_ADD(dst[i], dstStride[i] >> 1);
     }
 }
 
@@ -729,8 +729,8 @@ static int Rgb16ToPlanarRgb16Wrapper(SwsContext *c, const uint8_t *src[],
     }
 
     for(i=0; i<4; i++) {
-        dst2013[i] += stride2013[i] * srcSliceY / 2;
-        dst1023[i] += stride1023[i] * srcSliceY / 2;
+        dst2013[i] = FF_PTR_ADD(dst2013[i], stride2013[i] * srcSliceY / 2);
+        dst1023[i] = FF_PTR_ADD(dst1023[i], stride1023[i] * srcSliceY / 2);
     }
 
     switch (c->srcFormat) {
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swscale/swscale_unscaled: Fix undefined NULL + 0

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1537 bytes --]

On Wed, Sep 28, 2022 at 08:39:09PM +0200, Andreas Rheinhardt wrote:
> Affected the fitsdec-gbrp16 FATE-test.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libswscale/swscale_unscaled.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
> index 8838cc8b53..0b97377934 100644
> --- a/libswscale/swscale_unscaled.c
> +++ b/libswscale/swscale_unscaled.c
> @@ -695,7 +695,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
>              }
>          }
>          for (i = 0; i < 4; i++)
> -            dst[i] += dstStride[i] >> 1;
> +            dst[i] = FF_PTR_ADD(dst[i], dstStride[i] >> 1);
>      }
>  }
>  
> @@ -729,8 +729,8 @@ static int Rgb16ToPlanarRgb16Wrapper(SwsContext *c, const uint8_t *src[],
>      }
>  
>      for(i=0; i<4; i++) {
> -        dst2013[i] += stride2013[i] * srcSliceY / 2;
> -        dst1023[i] += stride1023[i] * srcSliceY / 2;
> +        dst2013[i] = FF_PTR_ADD(dst2013[i], stride2013[i] * srcSliceY / 2);
> +        dst1023[i] = FF_PTR_ADD(dst1023[i], stride1023[i] * srcSliceY / 2);
>      }

is there a reason not to check the pointer in the loop ?
as in
for (i = 0; i < 4 && dst[i]; i++)

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swscale/swscale_unscaled: Fix undefined NULL + 0

[Andreas Rheinhardt]

Michael Niedermayer:
> On Wed, Sep 28, 2022 at 08:39:09PM +0200, Andreas Rheinhardt wrote:
>> Affected the fitsdec-gbrp16 FATE-test.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libswscale/swscale_unscaled.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
>> index 8838cc8b53..0b97377934 100644
>> --- a/libswscale/swscale_unscaled.c
>> +++ b/libswscale/swscale_unscaled.c
>> @@ -695,7 +695,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
>>              }
>>          }
>>          for (i = 0; i < 4; i++)
>> -            dst[i] += dstStride[i] >> 1;
>> +            dst[i] = FF_PTR_ADD(dst[i], dstStride[i] >> 1);
>>      }
>>  }
>>  
>> @@ -729,8 +729,8 @@ static int Rgb16ToPlanarRgb16Wrapper(SwsContext *c, const uint8_t *src[],
>>      }
>>  
>>      for(i=0; i<4; i++) {
>> -        dst2013[i] += stride2013[i] * srcSliceY / 2;
>> -        dst1023[i] += stride1023[i] * srcSliceY / 2;
>> +        dst2013[i] = FF_PTR_ADD(dst2013[i], stride2013[i] * srcSliceY / 2);
>> +        dst1023[i] = FF_PTR_ADD(dst1023[i], stride1023[i] * srcSliceY / 2);
>>      }
> 
> is there a reason not to check the pointer in the loop ?
> as in
> for (i = 0; i < 4 && dst[i]; i++)
> 

I consider NULL + 0 to be sane and would be happy to see it being
defined in a future version of the spec. So I don't like adding checks
to workaround the insanities of the spec. Notice that FF_PTR_ADD() is
designed to allow the compiler to optimize the check away.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swscale/swscale_unscaled: Fix undefined NULL + 0

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2603 bytes --]

On Wed, Sep 28, 2022 at 09:43:56PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Wed, Sep 28, 2022 at 08:39:09PM +0200, Andreas Rheinhardt wrote:
> >> Affected the fitsdec-gbrp16 FATE-test.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> >> ---
> >>  libswscale/swscale_unscaled.c | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
> >> index 8838cc8b53..0b97377934 100644
> >> --- a/libswscale/swscale_unscaled.c
> >> +++ b/libswscale/swscale_unscaled.c
> >> @@ -695,7 +695,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
> >>              }
> >>          }
> >>          for (i = 0; i < 4; i++)
> >> -            dst[i] += dstStride[i] >> 1;
> >> +            dst[i] = FF_PTR_ADD(dst[i], dstStride[i] >> 1);
> >>      }
> >>  }
> >>  
> >> @@ -729,8 +729,8 @@ static int Rgb16ToPlanarRgb16Wrapper(SwsContext *c, const uint8_t *src[],
> >>      }
> >>  
> >>      for(i=0; i<4; i++) {
> >> -        dst2013[i] += stride2013[i] * srcSliceY / 2;
> >> -        dst1023[i] += stride1023[i] * srcSliceY / 2;
> >> +        dst2013[i] = FF_PTR_ADD(dst2013[i], stride2013[i] * srcSliceY / 2);
> >> +        dst1023[i] = FF_PTR_ADD(dst1023[i], stride1023[i] * srcSliceY / 2);
> >>      }
> > 
> > is there a reason not to check the pointer in the loop ?
> > as in
> > for (i = 0; i < 4 && dst[i]; i++)
> > 
> 
> I consider NULL + 0 to be sane and would be happy to see it being
> defined in a future version of the spec. 

i agree but that will not help our code for a long time


> So I don't like adding checks
> to workaround the insanities of the spec. 

Ive seen this less as a workaround and more as a "only work on
the active/used pointers"
a dst[i] check is more used elsewhere in sws too
but i really dont have much of an oppinon

libswscale/slice.c:    for (i = 0; i < 4 && src[i] != NULL; ++i) {
libswscale/swscale.c:            for (i = 0; i < 4 && src2[i]; i++) {
libswscale/swscale.c:            for (i = 0; i < 4 && dst2[i]; i++) {
libswscale/swscale_unscaled.c:    for (plane = 0; plane < 4 && dst[plane] != NULL; plane++) {
libswscale/tests/swscale.c:    for (i = 0; i < 4 && dstStride[i]; i++)
libswscale/swscale.c:        for (int i = 0; i < FF_ARRAY_ELEMS(dst) && parent->frame_dst->data[i]; i++) {

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".