[FFmpeg-devel] [PATCH 1/3] swscale/swscale_unscaled: fix planarCopyWrapper() for yuv444p => yuva444p

[FFmpeg-devel] [PATCH 1/3] swscale/swscale_unscaled: fix planarCopyWrapper() for yuv444p => yuva444p

[Ramiro Polla]

Currently, planarCopyWrapper() assumes that src[3] must be NULL when
the source format has no alpha plane.

This commit updates the condition for filling the alpha plane based on
the number of components available in the source format as well.
---
 libswscale/swscale_unscaled.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
index b84120549e..307e5471a9 100644
--- a/libswscale/swscale_unscaled.c
+++ b/libswscale/swscale_unscaled.c
@@ -2221,7 +2221,7 @@ static int planarCopyWrapper(SwsInternal *c, const uint8_t *const src[],
 
         // ignore palette for GRAY8
         if (plane == 1 && desc_dst->nb_components < 3) continue;
-        if (!src[plane] || (plane == 1 && desc_src->nb_components < 3)) {
+        if (!src[plane] || (plane == 1 && desc_src->nb_components < 3) || (plane == 3 && desc_src->nb_components <= 3)) {
             if (is16BPS(c->opts.dst_format) || isNBPS(c->opts.dst_format)) {
                 fillPlane16(dst[plane], dstStride[plane], length, height, y,
                         plane == 3, desc_dst->comp[plane].depth,
-- 
2.30.2

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/3] swscale/swscale_unscaled: fix packed30togbra10() for formats with bpc between 9-14 bits

[Ramiro Polla]

Currently, packed30togbra10() always sets the alpha value to 0xFFFF,
without taking the bit depth into consideration.

This commit restricts the alpha value to the bit depth.
---
 libswscale/swscale_unscaled.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
index 307e5471a9..8d71a88c23 100644
--- a/libswscale/swscale_unscaled.c
+++ b/libswscale/swscale_unscaled.c
@@ -818,6 +818,7 @@ static void packed30togbra10(const uint8_t *src, int srcStride,
     int x, h, i;
     int dst_alpha = dst[3] != NULL;
     int scale_high = bpc - 10, scale_low = 10 - scale_high;
+    uint16_t alpha_val = (1U << bpc) - 1;
     for (h = 0; h < srcSliceH; h++) {
         uint32_t *src_line = (uint32_t *)(src + srcStride * h);
         unsigned component;
@@ -834,7 +835,7 @@ static void packed30togbra10(const uint8_t *src, int srcStride,
                     dst[1][x] = av_bswap16(component << scale_high | component >> scale_low);
                     component =  p        & 0x3FF;
                     dst[2][x] = av_bswap16(component << scale_high | component >> scale_low);
-                    dst[3][x] = 0xFFFF;
+                    dst[3][x] = av_bswap16(alpha_val);
                     src_line++;
                 }
             } else {
@@ -860,7 +861,7 @@ static void packed30togbra10(const uint8_t *src, int srcStride,
                     dst[1][x] = component << scale_high | component >> scale_low;
                     component =  p        & 0x3FF;
                     dst[2][x] = component << scale_high | component >> scale_low;
-                    dst[3][x] = 0xFFFF;
+                    dst[3][x] = alpha_val;
                     src_line++;
                 }
             } else {
-- 
2.30.2

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/3] swscale/swscale_unscaled: fix packed16togbra16() for formats with bpc between 9-14 bits

[Ramiro Polla]

Currently, packed16togbra16() always sets the alpha value to 0xFFFF,
without taking the bit depth into consideration.

This commit restricts the alpha value to the bit depth.
---
 libswscale/swscale_unscaled.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
index 8d71a88c23..488db43e0b 100644
--- a/libswscale/swscale_unscaled.c
+++ b/libswscale/swscale_unscaled.c
@@ -699,7 +699,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
                     dst[0][x] = av_bswap16(av_bswap16(*src_line++) >> shift);
                     dst[1][x] = av_bswap16(av_bswap16(*src_line++) >> shift);
                     dst[2][x] = av_bswap16(av_bswap16(*src_line++) >> shift);
-                    dst[3][x] = 0xFFFF;
+                    dst[3][x] = av_bswap16(0xFFFF >> shift);
                 }
             } else if (src_alpha) {
                 for (x = 0; x < width; x++) {
@@ -729,7 +729,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
                     dst[0][x] = av_bswap16(*src_line++ >> shift);
                     dst[1][x] = av_bswap16(*src_line++ >> shift);
                     dst[2][x] = av_bswap16(*src_line++ >> shift);
-                    dst[3][x] = 0xFFFF;
+                    dst[3][x] = av_bswap16(0xFFFF >> shift);
                 }
             } else if (src_alpha) {
                 for (x = 0; x < width; x++) {
@@ -759,7 +759,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
                     dst[0][x] = av_bswap16(*src_line++) >> shift;
                     dst[1][x] = av_bswap16(*src_line++) >> shift;
                     dst[2][x] = av_bswap16(*src_line++) >> shift;
-                    dst[3][x] = 0xFFFF;
+                    dst[3][x] = 0xFFFF >> shift;
                 }
             } else if (src_alpha) {
                 for (x = 0; x < width; x++) {
@@ -789,7 +789,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
                     dst[0][x] = *src_line++ >> shift;
                     dst[1][x] = *src_line++ >> shift;
                     dst[2][x] = *src_line++ >> shift;
-                    dst[3][x] = 0xFFFF;
+                    dst[3][x] = 0xFFFF >> shift;
                 }
             } else if (src_alpha) {
                 for (x = 0; x < width; x++) {
-- 
2.30.2

_______________________________________________
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 3/3] swscale/swscale_unscaled: fix packed16togbra16() for formats with bpc between 9-14 bits

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 2740 bytes --]

On 5/18/2025 5:52 PM, Ramiro Polla wrote:
> Currently, packed16togbra16() always sets the alpha value to 0xFFFF,
> without taking the bit depth into consideration.
> 
> This commit restricts the alpha value to the bit depth.

packed16togbra16() seems to only be called for BGR48 and BGRA64, both of 
which are 16bits, so this change is superfluous.

> ---
>   libswscale/swscale_unscaled.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
> index 8d71a88c23..488db43e0b 100644
> --- a/libswscale/swscale_unscaled.c
> +++ b/libswscale/swscale_unscaled.c
> @@ -699,7 +699,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
>                       dst[0][x] = av_bswap16(av_bswap16(*src_line++) >> shift);
>                       dst[1][x] = av_bswap16(av_bswap16(*src_line++) >> shift);
>                       dst[2][x] = av_bswap16(av_bswap16(*src_line++) >> shift);
> -                    dst[3][x] = 0xFFFF;
> +                    dst[3][x] = av_bswap16(0xFFFF >> shift);
>                   }
>               } else if (src_alpha) {
>                   for (x = 0; x < width; x++) {
> @@ -729,7 +729,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
>                       dst[0][x] = av_bswap16(*src_line++ >> shift);
>                       dst[1][x] = av_bswap16(*src_line++ >> shift);
>                       dst[2][x] = av_bswap16(*src_line++ >> shift);
> -                    dst[3][x] = 0xFFFF;
> +                    dst[3][x] = av_bswap16(0xFFFF >> shift);
>                   }
>               } else if (src_alpha) {
>                   for (x = 0; x < width; x++) {
> @@ -759,7 +759,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
>                       dst[0][x] = av_bswap16(*src_line++) >> shift;
>                       dst[1][x] = av_bswap16(*src_line++) >> shift;
>                       dst[2][x] = av_bswap16(*src_line++) >> shift;
> -                    dst[3][x] = 0xFFFF;
> +                    dst[3][x] = 0xFFFF >> shift;
>                   }
>               } else if (src_alpha) {
>                   for (x = 0; x < width; x++) {
> @@ -789,7 +789,7 @@ static void packed16togbra16(const uint8_t *src, int srcStride,
>                       dst[0][x] = *src_line++ >> shift;
>                       dst[1][x] = *src_line++ >> shift;
>                       dst[2][x] = *src_line++ >> shift;
> -                    dst[3][x] = 0xFFFF;
> +                    dst[3][x] = 0xFFFF >> shift;
>                   }
>               } else if (src_alpha) {
>                   for (x = 0; x < width; x++) {


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 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 3/3] swscale/swscale_unscaled: fix packed16togbra16() for formats with bpc between 9-14 bits

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 3656 bytes --]

On 5/18/2025 6:14 PM, James Almer wrote:
> On 5/18/2025 5:52 PM, Ramiro Polla wrote:
>> Currently, packed16togbra16() always sets the alpha value to 0xFFFF,
>> without taking the bit depth into consideration.
>>
>> This commit restricts the alpha value to the bit depth.
> 
> packed16togbra16() seems to only be called for BGR48 and BGRA64, both of 
> which are 16bits, so this change is superfluous.

Ah, nevermind, i was looking at the src formats, not dst.

Are there no tests that cover these paths? I added a bunch a couple 
months ago, so maybe it could be extended.

> 
>> ---
>>   libswscale/swscale_unscaled.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/libswscale/swscale_unscaled.c b/libswscale/ 
>> swscale_unscaled.c
>> index 8d71a88c23..488db43e0b 100644
>> --- a/libswscale/swscale_unscaled.c
>> +++ b/libswscale/swscale_unscaled.c
>> @@ -699,7 +699,7 @@ static void packed16togbra16(const uint8_t *src, 
>> int srcStride,
>>                       dst[0][x] = av_bswap16(av_bswap16(*src_line++) 
>> >> shift);
>>                       dst[1][x] = av_bswap16(av_bswap16(*src_line++) 
>> >> shift);
>>                       dst[2][x] = av_bswap16(av_bswap16(*src_line++) 
>> >> shift);
>> -                    dst[3][x] = 0xFFFF;
>> +                    dst[3][x] = av_bswap16(0xFFFF >> shift);
>>                   }
>>               } else if (src_alpha) {
>>                   for (x = 0; x < width; x++) {
>> @@ -729,7 +729,7 @@ static void packed16togbra16(const uint8_t *src, 
>> int srcStride,
>>                       dst[0][x] = av_bswap16(*src_line++ >> shift);
>>                       dst[1][x] = av_bswap16(*src_line++ >> shift);
>>                       dst[2][x] = av_bswap16(*src_line++ >> shift);
>> -                    dst[3][x] = 0xFFFF;
>> +                    dst[3][x] = av_bswap16(0xFFFF >> shift);
>>                   }
>>               } else if (src_alpha) {
>>                   for (x = 0; x < width; x++) {
>> @@ -759,7 +759,7 @@ static void packed16togbra16(const uint8_t *src, 
>> int srcStride,
>>                       dst[0][x] = av_bswap16(*src_line++) >> shift;
>>                       dst[1][x] = av_bswap16(*src_line++) >> shift;
>>                       dst[2][x] = av_bswap16(*src_line++) >> shift;
>> -                    dst[3][x] = 0xFFFF;
>> +                    dst[3][x] = 0xFFFF >> shift;
>>                   }
>>               } else if (src_alpha) {
>>                   for (x = 0; x < width; x++) {
>> @@ -789,7 +789,7 @@ static void packed16togbra16(const uint8_t *src, 
>> int srcStride,
>>                       dst[0][x] = *src_line++ >> shift;
>>                       dst[1][x] = *src_line++ >> shift;
>>                       dst[2][x] = *src_line++ >> shift;
>> -                    dst[3][x] = 0xFFFF;
>> +                    dst[3][x] = 0xFFFF >> shift;
>>                   }
>>               } else if (src_alpha) {
>>                   for (x = 0; x < width; x++) {
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 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 3/3] swscale/swscale_unscaled: fix packed16togbra16() for formats with bpc between 9-14 bits

[Ramiro Polla]

On Sun, May 18, 2025 at 11:17 PM James Almer <jamrial@gmail.com> wrote:
> On 5/18/2025 6:14 PM, James Almer wrote:
> > On 5/18/2025 5:52 PM, Ramiro Polla wrote:
> >> Currently, packed16togbra16() always sets the alpha value to 0xFFFF,
> >> without taking the bit depth into consideration.
> >>
> >> This commit restricts the alpha value to the bit depth.
> >
> > packed16togbra16() seems to only be called for BGR48 and BGRA64, both of
> > which are 16bits, so this change is superfluous.
>
> Ah, nevermind, i was looking at the src formats, not dst.
>
> Are there no tests that cover these paths? I added a bunch a couple
> months ago, so maybe it could be extended.

It can be reproduced with:
./libswscale/tests/swscale -unscaled 1 -src xyz12le -dst gbrap12be

A little bit more information: this bug only happens on x86. The
problem arises from the optimized conversion that comes afterwards,
from gbrap12be to yuva444p, in ff_hscale14to15_4_ssse3(). It has
something to do with pmaddwd not working on unsigned values IIRC. We
could fix ff_hscale14to15_4_ssse3() to also work correctly with 0xFFFF
on bit depths < 16, or we could just not write 0xFFFF there in the
first place, which is what this patch does.

I thought about adding libswscale/tests/swscale to FATE, but the tests
take way too long.

Ramiro
_______________________________________________
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 3/3] swscale/swscale_unscaled: fix packed16togbra16() for formats with bpc between 9-14 bits

[Ramiro Polla]

On Mon, May 19, 2025 at 12:02 AM Ramiro Polla <ramiro.polla@gmail.com> wrote:
> On Sun, May 18, 2025 at 11:17 PM James Almer <jamrial@gmail.com> wrote:
> > On 5/18/2025 6:14 PM, James Almer wrote:
> > > On 5/18/2025 5:52 PM, Ramiro Polla wrote:
> > >> Currently, packed16togbra16() always sets the alpha value to 0xFFFF,
> > >> without taking the bit depth into consideration.
> > >>
> > >> This commit restricts the alpha value to the bit depth.
> > >
> > > packed16togbra16() seems to only be called for BGR48 and BGRA64, both of
> > > which are 16bits, so this change is superfluous.
> >
> > Ah, nevermind, i was looking at the src formats, not dst.
> >
> > Are there no tests that cover these paths? I added a bunch a couple
> > months ago, so maybe it could be extended.
>
> It can be reproduced with:
> ./libswscale/tests/swscale -unscaled 1 -src xyz12le -dst gbrap12be
>
> A little bit more information: this bug only happens on x86. The
> problem arises from the optimized conversion that comes afterwards,
> from gbrap12be to yuva444p, in ff_hscale14to15_4_ssse3(). It has
> something to do with pmaddwd not working on unsigned values IIRC. We
> could fix ff_hscale14to15_4_ssse3() to also work correctly with 0xFFFF
> on bit depths < 16, or we could just not write 0xFFFF there in the
> first place, which is what this patch does.
>
> I thought about adding libswscale/tests/swscale to FATE, but the tests
> take way too long.

I'll apply this patchset and the other similar patch about
planarRgbToplanarRgbWrapper() tomorrow if there are no more comments.

Ramiro
_______________________________________________
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".