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

[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 +0000, 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 <jc at kynesim.co.uk>
>> > ---
>> >  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 @@ void    rgb12to15(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 ths UV sample before RGB->UV

I can get to simple bilinear without adding so much complexity that I
lose the speed I need - would that be OK?

Ta

>thx
>
>[...]