Re: [FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion

2023-08-22 Thread Michael Niedermayer
On Tue, Aug 22, 2023 at 03:24:17PM +0100, John Cox wrote:
> On Mon, 21 Aug 2023 21:15:37 +0200, you wrote:
[...]
> >> I can get to simple bilinear without adding so much complexity that I
> >> lose the speed I need - would that be OK?
> >
> >Not sure simple bilinear is 100% clearly defined
> >I think it could mean 3 things
> >
> >1 2 1
> >  C
> >1 2 1
> >
> >or
> >
> >  1
> >  C
> >  1
> >
> >  or
> >
> >1 2 1
> >
> >3 6 3
> >  C
> >3 6 3
> >
> >1 2 1
> >
> >I think the 6 and 12 tap cases would produce ok results teh 2 tap not
> >Also maybe there are more finetuned filters for this specific case, i dont
> >know / didnt look.
> >Testing these probably would not be a bad idea before implementation
> >
> >I think users in 2023 expect the default to be better than what the
> >existing code was doing by default
> >so feel free to replace the existing "identical" code too
> 
> I was thinking of 2-tap (in both X & Y) which is equivalent to
> SWS_FAST_BILINEAR in ffmpeg. In the case I'm looking at I need the speed
> more than I need the quality and I'm quite happy to gate them behind a
> test for SWS_FAST_BILINEAR.

ok but maybe you want to still fix/add the higher quality C version too?
it would be almost the same code, just a different mix of source samples
iam asking as you are already working on this

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion

2023-08-22 Thread John Cox
On Mon, 21 Aug 2023 21:15:37 +0200, you wrote:

>On Sun, Aug 20, 2023 at 07:28:40PM +0100, John Cox wrote:
>> On Sun, 20 Aug 2023 19:45:11 +0200, you wrote:
>> 
>> >On Sun, Aug 20, 2023 at 07:16:14PM +0200, Michael Niedermayer wrote:
>> >> On Sun, Aug 20, 2023 at 03:10:19PM +, John Cox wrote:
>> >> > Add a rgb24->yuv420p conversion. Uses the same code as the existing
>> >> > bgr24->yuv converter but permutes the conversion array to swap R & B
>> >> > coefficients.
>> >> > 
>> >> > Signed-off-by: John Cox 
>> >> > ---
>> >> >  libswscale/rgb2rgb.c  |  5 +
>> >> >  libswscale/rgb2rgb.h  |  7 +++
>> >> >  libswscale/rgb2rgb_template.c | 38 ++-
>> >> >  libswscale/swscale_unscaled.c | 24 +-
>> >> >  4 files changed, 68 insertions(+), 6 deletions(-)
>> >> > 
>> >> > diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
>> >> > index 8707917800..de90e5193f 100644
>> >> > --- a/libswscale/rgb2rgb.c
>> >> > +++ b/libswscale/rgb2rgb.c
>> >> > @@ -83,6 +83,11 @@ void (*ff_bgr24toyv12)(const uint8_t *src, uint8_t 
>> >> > *ydst,
>> >> > int width, int height,
>> >> > int lumStride, int chromStride, int srcStride,
>> >> > int32_t *rgb2yuv);
>> >> > +void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst,
>> >> > +   uint8_t *udst, uint8_t *vdst,
>> >> > +   int width, int height,
>> >> > +   int lumStride, int chromStride, int srcStride,
>> >> > +   int32_t *rgb2yuv);
>> >> >  void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int 
>> >> > height,
>> >> >   int srcStride, int dstStride);
>> >> >  void (*interleaveBytes)(const uint8_t *src1, const uint8_t *src2, 
>> >> > uint8_t *dst,
>> >> > diff --git a/libswscale/rgb2rgb.h b/libswscale/rgb2rgb.h
>> >> > index 305b830920..f7a76a92ba 100644
>> >> > --- a/libswscale/rgb2rgb.h
>> >> > +++ b/libswscale/rgb2rgb.h
>> >> > @@ -79,6 +79,9 @@ voidrgb12to15(const uint8_t *src, uint8_t *dst, 
>> >> > int src_size);
>> >> >  void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>> >> >uint8_t *vdst, int width, int height, int 
>> >> > lumStride,
>> >> >int chromStride, int srcStride, int32_t 
>> >> > *rgb2yuv);
>> >> > +void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>> >> > +  uint8_t *vdst, int width, int height, int 
>> >> > lumStride,
>> >> > +  int chromStride, int srcStride, int32_t 
>> >> > *rgb2yuv);
>> >> >  
>> >> >  /**
>> >> >   * Height should be a multiple of 2 and width should be a multiple of 
>> >> > 16.
>> >> > @@ -128,6 +131,10 @@ extern void (*ff_bgr24toyv12)(const uint8_t *src, 
>> >> > uint8_t *ydst, uint8_t *udst,
>> >> >int width, int height,
>> >> >int lumStride, int chromStride, int 
>> >> > srcStride,
>> >> >int32_t *rgb2yuv);
>> >> > +extern void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst, 
>> >> > uint8_t *udst, uint8_t *vdst,
>> >> > +  int width, int height,
>> >> > +  int lumStride, int chromStride, int 
>> >> > srcStride,
>> >> > +  int32_t *rgb2yuv);
>> >> >  extern void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, 
>> >> > int height,
>> >> >  int srcStride, int dstStride);
>> >> >  
>> >> > diff --git a/libswscale/rgb2rgb_template.c 
>> >> > b/libswscale/rgb2rgb_template.c
>> >> > index 8ef4a2cf5d..e57bfa6545 100644
>> >> > --- a/libswscale/rgb2rgb_template.c
>> >> > +++ b/libswscale/rgb2rgb_template.c
>> >> 
>> >> 
>> >> > @@ -646,13 +646,14 @@ static inline void uyvytoyv12_c(const uint8_t 
>> >> > *src, uint8_t *ydst,
>> >> >   * others are ignored in the C version.
>> >> >   * FIXME: Write HQ version.
>> >> >   */
>> >> > -void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>> >> > +static void rgb24toyv12_x(const uint8_t *src, uint8_t *ydst, uint8_t 
>> >> > *udst,
>> >> 
>> >> this probably should be inline
>> >> 
>> >> also i see now "FIXME: Write HQ version." above here. Do you really want 
>> >> to
>> >> add a low quality rgb24toyv12 ?
>> >> (it is vissible on the diagonal border (cyan / red )) in
>> >>  ./ffmpeg -f lavfi -i testsrc=size=5632x3168 -pix_fmt yuv420p -vframes 1 
>> >> -qscale 1 -strict -1 new.jpg
>> >> 
>> >>  also on smaller sizes but for some reason its clearer on the big one 
>> >> zoomed in 400% with gimp
>> >> (the gimp test was done with the whole patchset not after this patch)
>> >
>> >Also the reason why its LQ and looks like it does is because
>> >1. half the RGB samples are ignored in computing the chroma samples
>> 
>> I thought it was a bit light but it is what the existing code 

Re: [FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion

2023-08-21 Thread Michael Niedermayer
On Sun, Aug 20, 2023 at 07:28:40PM +0100, John Cox wrote:
> On Sun, 20 Aug 2023 19:45:11 +0200, you wrote:
> 
> >On Sun, Aug 20, 2023 at 07:16:14PM +0200, Michael Niedermayer wrote:
> >> On Sun, Aug 20, 2023 at 03:10:19PM +, John Cox wrote:
> >> > Add a rgb24->yuv420p conversion. Uses the same code as the existing
> >> > bgr24->yuv converter but permutes the conversion array to swap R & B
> >> > coefficients.
> >> > 
> >> > Signed-off-by: John Cox 
> >> > ---
> >> >  libswscale/rgb2rgb.c  |  5 +
> >> >  libswscale/rgb2rgb.h  |  7 +++
> >> >  libswscale/rgb2rgb_template.c | 38 ++-
> >> >  libswscale/swscale_unscaled.c | 24 +-
> >> >  4 files changed, 68 insertions(+), 6 deletions(-)
> >> > 
> >> > diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
> >> > index 8707917800..de90e5193f 100644
> >> > --- a/libswscale/rgb2rgb.c
> >> > +++ b/libswscale/rgb2rgb.c
> >> > @@ -83,6 +83,11 @@ void (*ff_bgr24toyv12)(const uint8_t *src, uint8_t 
> >> > *ydst,
> >> > int width, int height,
> >> > int lumStride, int chromStride, int srcStride,
> >> > int32_t *rgb2yuv);
> >> > +void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst,
> >> > +   uint8_t *udst, uint8_t *vdst,
> >> > +   int width, int height,
> >> > +   int lumStride, int chromStride, int srcStride,
> >> > +   int32_t *rgb2yuv);
> >> >  void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int 
> >> > height,
> >> >   int srcStride, int dstStride);
> >> >  void (*interleaveBytes)(const uint8_t *src1, const uint8_t *src2, 
> >> > uint8_t *dst,
> >> > diff --git a/libswscale/rgb2rgb.h b/libswscale/rgb2rgb.h
> >> > index 305b830920..f7a76a92ba 100644
> >> > --- a/libswscale/rgb2rgb.h
> >> > +++ b/libswscale/rgb2rgb.h
> >> > @@ -79,6 +79,9 @@ voidrgb12to15(const uint8_t *src, uint8_t *dst, 
> >> > int src_size);
> >> >  void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> >> >uint8_t *vdst, int width, int height, int 
> >> > lumStride,
> >> >int chromStride, int srcStride, int32_t *rgb2yuv);
> >> > +void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> >> > +  uint8_t *vdst, int width, int height, int 
> >> > lumStride,
> >> > +  int chromStride, int srcStride, int32_t *rgb2yuv);
> >> >  
> >> >  /**
> >> >   * Height should be a multiple of 2 and width should be a multiple of 
> >> > 16.
> >> > @@ -128,6 +131,10 @@ extern void (*ff_bgr24toyv12)(const uint8_t *src, 
> >> > uint8_t *ydst, uint8_t *udst,
> >> >int width, int height,
> >> >int lumStride, int chromStride, int 
> >> > srcStride,
> >> >int32_t *rgb2yuv);
> >> > +extern void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst, 
> >> > uint8_t *udst, uint8_t *vdst,
> >> > +  int width, int height,
> >> > +  int lumStride, int chromStride, int 
> >> > srcStride,
> >> > +  int32_t *rgb2yuv);
> >> >  extern void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, 
> >> > int height,
> >> >  int srcStride, int dstStride);
> >> >  
> >> > diff --git a/libswscale/rgb2rgb_template.c 
> >> > b/libswscale/rgb2rgb_template.c
> >> > index 8ef4a2cf5d..e57bfa6545 100644
> >> > --- a/libswscale/rgb2rgb_template.c
> >> > +++ b/libswscale/rgb2rgb_template.c
> >> 
> >> 
> >> > @@ -646,13 +646,14 @@ static inline void uyvytoyv12_c(const uint8_t 
> >> > *src, uint8_t *ydst,
> >> >   * others are ignored in the C version.
> >> >   * FIXME: Write HQ version.
> >> >   */
> >> > -void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> >> > +static void rgb24toyv12_x(const uint8_t *src, uint8_t *ydst, uint8_t 
> >> > *udst,
> >> 
> >> this probably should be inline
> >> 
> >> also i see now "FIXME: Write HQ version." above here. Do you really want to
> >> add a low quality rgb24toyv12 ?
> >> (it is vissible on the diagonal border (cyan / red )) in
> >>  ./ffmpeg -f lavfi -i testsrc=size=5632x3168 -pix_fmt yuv420p -vframes 1 
> >> -qscale 1 -strict -1 new.jpg
> >> 
> >>  also on smaller sizes but for some reason its clearer on the big one 
> >> zoomed in 400% with gimp
> >> (the gimp test was done with the whole patchset not after this patch)
> >
> >Also the reason why its LQ and looks like it does is because
> >1. half the RGB samples are ignored in computing the chroma samples
> 
> I thought it was a bit light but it is what the existing code did
> 
> >2. the chroma sample locations are ignored, the locations for yuv420 are 
> >reaonable standard
> 
> As I recall MPEG-1 has chroma at (0.5, 0.5), MPEG-II defaults 

Re: [FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion

2023-08-20 Thread John Cox
On Sun, 20 Aug 2023 19:45:11 +0200, you wrote:

>On Sun, Aug 20, 2023 at 07:16:14PM +0200, Michael Niedermayer wrote:
>> On Sun, Aug 20, 2023 at 03:10:19PM +, John Cox wrote:
>> > Add a rgb24->yuv420p conversion. Uses the same code as the existing
>> > bgr24->yuv converter but permutes the conversion array to swap R & B
>> > coefficients.
>> > 
>> > Signed-off-by: John Cox 
>> > ---
>> >  libswscale/rgb2rgb.c  |  5 +
>> >  libswscale/rgb2rgb.h  |  7 +++
>> >  libswscale/rgb2rgb_template.c | 38 ++-
>> >  libswscale/swscale_unscaled.c | 24 +-
>> >  4 files changed, 68 insertions(+), 6 deletions(-)
>> > 
>> > diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
>> > index 8707917800..de90e5193f 100644
>> > --- a/libswscale/rgb2rgb.c
>> > +++ b/libswscale/rgb2rgb.c
>> > @@ -83,6 +83,11 @@ void (*ff_bgr24toyv12)(const uint8_t *src, uint8_t 
>> > *ydst,
>> > int width, int height,
>> > int lumStride, int chromStride, int srcStride,
>> > int32_t *rgb2yuv);
>> > +void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst,
>> > +   uint8_t *udst, uint8_t *vdst,
>> > +   int width, int height,
>> > +   int lumStride, int chromStride, int srcStride,
>> > +   int32_t *rgb2yuv);
>> >  void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int height,
>> >   int srcStride, int dstStride);
>> >  void (*interleaveBytes)(const uint8_t *src1, const uint8_t *src2, uint8_t 
>> > *dst,
>> > diff --git a/libswscale/rgb2rgb.h b/libswscale/rgb2rgb.h
>> > index 305b830920..f7a76a92ba 100644
>> > --- a/libswscale/rgb2rgb.h
>> > +++ b/libswscale/rgb2rgb.h
>> > @@ -79,6 +79,9 @@ voidrgb12to15(const uint8_t *src, uint8_t *dst, int 
>> > src_size);
>> >  void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>> >uint8_t *vdst, int width, int height, int lumStride,
>> >int chromStride, int srcStride, int32_t *rgb2yuv);
>> > +void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>> > +  uint8_t *vdst, int width, int height, int lumStride,
>> > +  int chromStride, int srcStride, int32_t *rgb2yuv);
>> >  
>> >  /**
>> >   * Height should be a multiple of 2 and width should be a multiple of 16.
>> > @@ -128,6 +131,10 @@ extern void (*ff_bgr24toyv12)(const uint8_t *src, 
>> > uint8_t *ydst, uint8_t *udst,
>> >int width, int height,
>> >int lumStride, int chromStride, int 
>> > srcStride,
>> >int32_t *rgb2yuv);
>> > +extern void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst, uint8_t 
>> > *udst, uint8_t *vdst,
>> > +  int width, int height,
>> > +  int lumStride, int chromStride, int 
>> > srcStride,
>> > +  int32_t *rgb2yuv);
>> >  extern void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int 
>> > height,
>> >  int srcStride, int dstStride);
>> >  
>> > diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_template.c
>> > index 8ef4a2cf5d..e57bfa6545 100644
>> > --- a/libswscale/rgb2rgb_template.c
>> > +++ b/libswscale/rgb2rgb_template.c
>> 
>> 
>> > @@ -646,13 +646,14 @@ static inline void uyvytoyv12_c(const uint8_t *src, 
>> > uint8_t *ydst,
>> >   * others are ignored in the C version.
>> >   * FIXME: Write HQ version.
>> >   */
>> > -void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>> > +static void rgb24toyv12_x(const uint8_t *src, uint8_t *ydst, uint8_t 
>> > *udst,
>> 
>> this probably should be inline
>> 
>> also i see now "FIXME: Write HQ version." above here. Do you really want to
>> add a low quality rgb24toyv12 ?
>> (it is vissible on the diagonal border (cyan / red )) in
>>  ./ffmpeg -f lavfi -i testsrc=size=5632x3168 -pix_fmt yuv420p -vframes 1 
>> -qscale 1 -strict -1 new.jpg
>> 
>>  also on smaller sizes but for some reason its clearer on the big one zoomed 
>> in 400% with gimp
>> (the gimp test was done with the whole patchset not after this patch)
>
>Also the reason why its LQ and looks like it does is because
>1. half the RGB samples are ignored in computing the chroma samples

I thought it was a bit light but it is what the existing code did

>2. the chroma sample locations are ignored, the locations for yuv420 are 
>reaonable standard

As I recall MPEG-1 has chroma at (0.5, 0.5), MPEG-II defaults to (0.5,
0), H.265 defaults to (0,0). Printing out dst_h_chr_pos, dst_v_chr_pos
in the setup of your example yields -513, 128 which I'm guessing means
(unset, 0.5) - am I looking at the correct vars?

>this needs some simple filter to get from a few RGB samples to the RGB sample 
>co-located
>with 

Re: [FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion

2023-08-20 Thread John Cox
On Sun, 20 Aug 2023 19:16:14 +0200, you wrote:

>On Sun, Aug 20, 2023 at 03:10:19PM +, John Cox wrote:
>> Add a rgb24->yuv420p conversion. Uses the same code as the existing
>> bgr24->yuv converter but permutes the conversion array to swap R & B
>> coefficients.
>> 
>> Signed-off-by: John Cox 
>> ---
>>  libswscale/rgb2rgb.c  |  5 +
>>  libswscale/rgb2rgb.h  |  7 +++
>>  libswscale/rgb2rgb_template.c | 38 ++-
>>  libswscale/swscale_unscaled.c | 24 +-
>>  4 files changed, 68 insertions(+), 6 deletions(-)
>> 
>> diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
>> index 8707917800..de90e5193f 100644
>> --- a/libswscale/rgb2rgb.c
>> +++ b/libswscale/rgb2rgb.c
>> @@ -83,6 +83,11 @@ void (*ff_bgr24toyv12)(const uint8_t *src, uint8_t *ydst,
>> int width, int height,
>> int lumStride, int chromStride, int srcStride,
>> int32_t *rgb2yuv);
>> +void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst,
>> +   uint8_t *udst, uint8_t *vdst,
>> +   int width, int height,
>> +   int lumStride, int chromStride, int srcStride,
>> +   int32_t *rgb2yuv);
>>  void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int height,
>>   int srcStride, int dstStride);
>>  void (*interleaveBytes)(const uint8_t *src1, const uint8_t *src2, uint8_t 
>> *dst,
>> diff --git a/libswscale/rgb2rgb.h b/libswscale/rgb2rgb.h
>> index 305b830920..f7a76a92ba 100644
>> --- a/libswscale/rgb2rgb.h
>> +++ b/libswscale/rgb2rgb.h
>> @@ -79,6 +79,9 @@ voidrgb12to15(const uint8_t *src, uint8_t *dst, int 
>> src_size);
>>  void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>>uint8_t *vdst, int width, int height, int lumStride,
>>int chromStride, int srcStride, int32_t *rgb2yuv);
>> +void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>> +  uint8_t *vdst, int width, int height, int lumStride,
>> +  int chromStride, int srcStride, int32_t *rgb2yuv);
>>  
>>  /**
>>   * Height should be a multiple of 2 and width should be a multiple of 16.
>> @@ -128,6 +131,10 @@ extern void (*ff_bgr24toyv12)(const uint8_t *src, 
>> uint8_t *ydst, uint8_t *udst,
>>int width, int height,
>>int lumStride, int chromStride, int srcStride,
>>int32_t *rgb2yuv);
>> +extern void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst, uint8_t 
>> *udst, uint8_t *vdst,
>> +  int width, int height,
>> +  int lumStride, int chromStride, int srcStride,
>> +  int32_t *rgb2yuv);
>>  extern void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int 
>> height,
>>  int srcStride, int dstStride);
>>  
>> diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_template.c
>> index 8ef4a2cf5d..e57bfa6545 100644
>> --- a/libswscale/rgb2rgb_template.c
>> +++ b/libswscale/rgb2rgb_template.c
>
>
>> @@ -646,13 +646,14 @@ static inline void uyvytoyv12_c(const uint8_t *src, 
>> uint8_t *ydst,
>>   * others are ignored in the C version.
>>   * FIXME: Write HQ version.
>>   */
>> -void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>> +static void rgb24toyv12_x(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>
>this probably should be inline

Could do, and I will if you deem it important, but the only bit that
inline is going to help is the matrix coefficient loading and that
happens once outside the main loops.

>also i see now "FIXME: Write HQ version." above here. Do you really want to
>add a low quality rgb24toyv12 ?
>(it is vissible on the diagonal border (cyan / red )) in
> ./ffmpeg -f lavfi -i testsrc=size=5632x3168 -pix_fmt yuv420p -vframes 1 
> -qscale 1 -strict -1 new.jpg
>
> also on smaller sizes but for some reason its clearer on the big one zoomed 
> in 400% with gimp
>(the gimp test was done with the whole patchset not after this patch)

On the whole - yes - in the encode path on the Pi that I'm writing this
for speed is more important than quality - the existing path is too slow
to be usable. And honestly - using your example above comparing (Windows
photo viewer zoomed in s.t. pixels are clearly individually visible) the
general (bitexact), presumably HQ, output vs the new code I grant that
the new is slightly muckier but not by a huge amount - sharp chroma
transitions in 420 are always nasty.

>[...]
>> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
>> index 32e0d7f63c..751bdcb2e4 100644
>> --- a/libswscale/swscale_unscaled.c
>> +++ b/libswscale/swscale_unscaled.c
>> @@ -1654,6 +1654,23 @@ static int bgr24ToYv12Wrapper(SwsContext *c, const 

Re: [FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion

2023-08-20 Thread Michael Niedermayer
On Sun, Aug 20, 2023 at 07:16:14PM +0200, Michael Niedermayer wrote:
> On Sun, Aug 20, 2023 at 03:10:19PM +, John Cox wrote:
> > Add a rgb24->yuv420p conversion. Uses the same code as the existing
> > bgr24->yuv converter but permutes the conversion array to swap R & B
> > coefficients.
> > 
> > Signed-off-by: John Cox 
> > ---
> >  libswscale/rgb2rgb.c  |  5 +
> >  libswscale/rgb2rgb.h  |  7 +++
> >  libswscale/rgb2rgb_template.c | 38 ++-
> >  libswscale/swscale_unscaled.c | 24 +-
> >  4 files changed, 68 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
> > index 8707917800..de90e5193f 100644
> > --- a/libswscale/rgb2rgb.c
> > +++ b/libswscale/rgb2rgb.c
> > @@ -83,6 +83,11 @@ void (*ff_bgr24toyv12)(const uint8_t *src, uint8_t *ydst,
> > int width, int height,
> > int lumStride, int chromStride, int srcStride,
> > int32_t *rgb2yuv);
> > +void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst,
> > +   uint8_t *udst, uint8_t *vdst,
> > +   int width, int height,
> > +   int lumStride, int chromStride, int srcStride,
> > +   int32_t *rgb2yuv);
> >  void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int height,
> >   int srcStride, int dstStride);
> >  void (*interleaveBytes)(const uint8_t *src1, const uint8_t *src2, uint8_t 
> > *dst,
> > diff --git a/libswscale/rgb2rgb.h b/libswscale/rgb2rgb.h
> > index 305b830920..f7a76a92ba 100644
> > --- a/libswscale/rgb2rgb.h
> > +++ b/libswscale/rgb2rgb.h
> > @@ -79,6 +79,9 @@ voidrgb12to15(const uint8_t *src, uint8_t *dst, int 
> > src_size);
> >  void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> >uint8_t *vdst, int width, int height, int lumStride,
> >int chromStride, int srcStride, int32_t *rgb2yuv);
> > +void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> > +  uint8_t *vdst, int width, int height, int lumStride,
> > +  int chromStride, int srcStride, int32_t *rgb2yuv);
> >  
> >  /**
> >   * Height should be a multiple of 2 and width should be a multiple of 16.
> > @@ -128,6 +131,10 @@ extern void (*ff_bgr24toyv12)(const uint8_t *src, 
> > uint8_t *ydst, uint8_t *udst,
> >int width, int height,
> >int lumStride, int chromStride, int 
> > srcStride,
> >int32_t *rgb2yuv);
> > +extern void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst, uint8_t 
> > *udst, uint8_t *vdst,
> > +  int width, int height,
> > +  int lumStride, int chromStride, int 
> > srcStride,
> > +  int32_t *rgb2yuv);
> >  extern void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int 
> > height,
> >  int srcStride, int dstStride);
> >  
> > diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_template.c
> > index 8ef4a2cf5d..e57bfa6545 100644
> > --- a/libswscale/rgb2rgb_template.c
> > +++ b/libswscale/rgb2rgb_template.c
> 
> 
> > @@ -646,13 +646,14 @@ static inline void uyvytoyv12_c(const uint8_t *src, 
> > uint8_t *ydst,
> >   * others are ignored in the C version.
> >   * FIXME: Write HQ version.
> >   */
> > -void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> > +static void rgb24toyv12_x(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> 
> this probably should be inline
> 
> also i see now "FIXME: Write HQ version." above here. Do you really want to
> add a low quality rgb24toyv12 ?
> (it is vissible on the diagonal border (cyan / red )) in
>  ./ffmpeg -f lavfi -i testsrc=size=5632x3168 -pix_fmt yuv420p -vframes 1 
> -qscale 1 -strict -1 new.jpg
> 
>  also on smaller sizes but for some reason its clearer on the big one zoomed 
> in 400% with gimp
> (the gimp test was done with the whole patchset not after this patch)

Also the reason why its LQ and looks like it does is because
1. half the RGB samples are ignored in computing the chroma samples
2. the chroma sample locations are ignored, the locations for yuv420 are 
reaonable standard

this needs some simple filter to get from a few RGB samples to the RGB sample 
co-located
with ths UV sample before RGB->UV

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion

2023-08-20 Thread Michael Niedermayer
On Sun, Aug 20, 2023 at 03:10:19PM +, John Cox wrote:
> Add a rgb24->yuv420p conversion. Uses the same code as the existing
> bgr24->yuv converter but permutes the conversion array to swap R & B
> coefficients.
> 
> Signed-off-by: John Cox 
> ---
>  libswscale/rgb2rgb.c  |  5 +
>  libswscale/rgb2rgb.h  |  7 +++
>  libswscale/rgb2rgb_template.c | 38 ++-
>  libswscale/swscale_unscaled.c | 24 +-
>  4 files changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
> index 8707917800..de90e5193f 100644
> --- a/libswscale/rgb2rgb.c
> +++ b/libswscale/rgb2rgb.c
> @@ -83,6 +83,11 @@ void (*ff_bgr24toyv12)(const uint8_t *src, uint8_t *ydst,
> int width, int height,
> int lumStride, int chromStride, int srcStride,
> int32_t *rgb2yuv);
> +void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst,
> +   uint8_t *udst, uint8_t *vdst,
> +   int width, int height,
> +   int lumStride, int chromStride, int srcStride,
> +   int32_t *rgb2yuv);
>  void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int height,
>   int srcStride, int dstStride);
>  void (*interleaveBytes)(const uint8_t *src1, const uint8_t *src2, uint8_t 
> *dst,
> diff --git a/libswscale/rgb2rgb.h b/libswscale/rgb2rgb.h
> index 305b830920..f7a76a92ba 100644
> --- a/libswscale/rgb2rgb.h
> +++ b/libswscale/rgb2rgb.h
> @@ -79,6 +79,9 @@ voidrgb12to15(const uint8_t *src, uint8_t *dst, int 
> src_size);
>  void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
>uint8_t *vdst, int width, int height, int lumStride,
>int chromStride, int srcStride, int32_t *rgb2yuv);
> +void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> +  uint8_t *vdst, int width, int height, int lumStride,
> +  int chromStride, int srcStride, int32_t *rgb2yuv);
>  
>  /**
>   * Height should be a multiple of 2 and width should be a multiple of 16.
> @@ -128,6 +131,10 @@ extern void (*ff_bgr24toyv12)(const uint8_t *src, 
> uint8_t *ydst, uint8_t *udst,
>int width, int height,
>int lumStride, int chromStride, int srcStride,
>int32_t *rgb2yuv);
> +extern void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst, uint8_t 
> *udst, uint8_t *vdst,
> +  int width, int height,
> +  int lumStride, int chromStride, int srcStride,
> +  int32_t *rgb2yuv);
>  extern void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int 
> height,
>  int srcStride, int dstStride);
>  
> diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_template.c
> index 8ef4a2cf5d..e57bfa6545 100644
> --- a/libswscale/rgb2rgb_template.c
> +++ b/libswscale/rgb2rgb_template.c


> @@ -646,13 +646,14 @@ static inline void uyvytoyv12_c(const uint8_t *src, 
> uint8_t *ydst,
>   * others are ignored in the C version.
>   * FIXME: Write HQ version.
>   */
> -void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
> +static void rgb24toyv12_x(const uint8_t *src, uint8_t *ydst, uint8_t *udst,

this probably should be inline

also i see now "FIXME: Write HQ version." above here. Do you really want to
add a low quality rgb24toyv12 ?
(it is vissible on the diagonal border (cyan / red )) in
 ./ffmpeg -f lavfi -i testsrc=size=5632x3168 -pix_fmt yuv420p -vframes 1 
-qscale 1 -strict -1 new.jpg

 also on smaller sizes but for some reason its clearer on the big one zoomed in 
400% with gimp
(the gimp test was done with the whole patchset not after this patch)


[...]
> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
> index 32e0d7f63c..751bdcb2e4 100644
> --- a/libswscale/swscale_unscaled.c
> +++ b/libswscale/swscale_unscaled.c
> @@ -1654,6 +1654,23 @@ static int bgr24ToYv12Wrapper(SwsContext *c, const 
> uint8_t *src[],
>  return srcSliceH;
>  }
>  
> +static int rgb24ToYv12Wrapper(SwsContext *c, const uint8_t *src[],
> +  int srcStride[], int srcSliceY, int srcSliceH,
> +  uint8_t *dst[], int dstStride[])
> +{
> +ff_rgb24toyv12(
> +src[0],
> +dst[0] +  srcSliceY   * dstStride[0],
> +dst[1] + (srcSliceY >> 1) * dstStride[1],
> +dst[2] + (srcSliceY >> 1) * dstStride[2],
> +c->srcW, srcSliceH,
> +dstStride[0], dstStride[1], srcStride[0],
> +c->input_rgb2yuv_table);
> +if (dst[3])
> +fillPlane(dst[3], dstStride[3], c->srcW, srcSliceH, srcSliceY, 255);
> +return srcSliceH;
> +}
> +
>  static int yvu9ToYv12Wrapper(SwsContext *c, 

[FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion

2023-08-20 Thread John Cox
Add a rgb24->yuv420p conversion. Uses the same code as the existing
bgr24->yuv converter but permutes the conversion array to swap R & B
coefficients.

Signed-off-by: John Cox 
---
 libswscale/rgb2rgb.c  |  5 +
 libswscale/rgb2rgb.h  |  7 +++
 libswscale/rgb2rgb_template.c | 38 ++-
 libswscale/swscale_unscaled.c | 24 +-
 4 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
index 8707917800..de90e5193f 100644
--- a/libswscale/rgb2rgb.c
+++ b/libswscale/rgb2rgb.c
@@ -83,6 +83,11 @@ void (*ff_bgr24toyv12)(const uint8_t *src, uint8_t *ydst,
int width, int height,
int lumStride, int chromStride, int srcStride,
int32_t *rgb2yuv);
+void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst,
+   uint8_t *udst, uint8_t *vdst,
+   int width, int height,
+   int lumStride, int chromStride, int srcStride,
+   int32_t *rgb2yuv);
 void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int height,
  int srcStride, int dstStride);
 void (*interleaveBytes)(const uint8_t *src1, const uint8_t *src2, uint8_t *dst,
diff --git a/libswscale/rgb2rgb.h b/libswscale/rgb2rgb.h
index 305b830920..f7a76a92ba 100644
--- a/libswscale/rgb2rgb.h
+++ b/libswscale/rgb2rgb.h
@@ -79,6 +79,9 @@ voidrgb12to15(const uint8_t *src, uint8_t *dst, int 
src_size);
 void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
   uint8_t *vdst, int width, int height, int lumStride,
   int chromStride, int srcStride, int32_t *rgb2yuv);
+void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
+  uint8_t *vdst, int width, int height, int lumStride,
+  int chromStride, int srcStride, int32_t *rgb2yuv);
 
 /**
  * Height should be a multiple of 2 and width should be a multiple of 16.
@@ -128,6 +131,10 @@ extern void (*ff_bgr24toyv12)(const uint8_t *src, uint8_t 
*ydst, uint8_t *udst,
   int width, int height,
   int lumStride, int chromStride, int srcStride,
   int32_t *rgb2yuv);
+extern void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst, uint8_t 
*udst, uint8_t *vdst,
+  int width, int height,
+  int lumStride, int chromStride, int srcStride,
+  int32_t *rgb2yuv);
 extern void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int 
height,
 int srcStride, int dstStride);
 
diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_template.c
index 8ef4a2cf5d..e57bfa6545 100644
--- a/libswscale/rgb2rgb_template.c
+++ b/libswscale/rgb2rgb_template.c
@@ -646,13 +646,14 @@ static inline void uyvytoyv12_c(const uint8_t *src, 
uint8_t *ydst,
  * others are ignored in the C version.
  * FIXME: Write HQ version.
  */
-void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
+static void rgb24toyv12_x(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
uint8_t *vdst, int width, int height, int lumStride,
-   int chromStride, int srcStride, int32_t *rgb2yuv)
+   int chromStride, int srcStride, int32_t *rgb2yuv,
+   const uint8_t x[9])
 {
-int32_t ry = rgb2yuv[RY_IDX], gy = rgb2yuv[GY_IDX], by = rgb2yuv[BY_IDX];
-int32_t ru = rgb2yuv[RU_IDX], gu = rgb2yuv[GU_IDX], bu = rgb2yuv[BU_IDX];
-int32_t rv = rgb2yuv[RV_IDX], gv = rgb2yuv[GV_IDX], bv = rgb2yuv[BV_IDX];
+int32_t ry = rgb2yuv[x[0]], gy = rgb2yuv[x[1]], by = rgb2yuv[x[2]];
+int32_t ru = rgb2yuv[x[3]], gu = rgb2yuv[x[4]], bu = rgb2yuv[x[5]];
+int32_t rv = rgb2yuv[x[6]], gv = rgb2yuv[x[7]], bv = rgb2yuv[x[8]];
 int y;
 const int chromWidth = width >> 1;
 
@@ -707,6 +708,32 @@ void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, 
uint8_t *udst,
 }
 }
 
+static const uint8_t x_bgr[9] = {
+RY_IDX, GY_IDX, BY_IDX,
+RU_IDX, GU_IDX, BU_IDX,
+RV_IDX, GV_IDX, BV_IDX,
+};
+
+static const uint8_t x_rgb[9] = {
+ BY_IDX, GY_IDX, RY_IDX,
+ BU_IDX, GU_IDX, RU_IDX,
+ BV_IDX, GV_IDX, RV_IDX,
+};
+
+void ff_bgr24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
+   uint8_t *vdst, int width, int height, int lumStride,
+   int chromStride, int srcStride, int32_t *rgb2yuv)
+{
+rgb24toyv12_x(src, ydst, udst, vdst, width, height, lumStride, 
chromStride, srcStride, rgb2yuv, x_bgr);
+}
+
+void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *udst,
+   uint8_t *vdst, int width, int height, int lumStride,
+   int chromStride, int srcStride, int32_t *rgb2yuv)
+{
+rgb24toyv12_x(src, ydst, udst,