[FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion
John Cox
jc at kynesim.co.uk
Tue Aug 22 17:24:17 EEST 2023
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 +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),
>
>yes
>
>
>> H.265 defaults to (0,0).
>
>hmm
> When the value of chroma_format_idc is equal to 1, the nominal vertical and horizontal relative locations of luma and
> chroma samples in pictures are shown in Figure 6-1. Alternative chroma sample relative locations may be indicated in
> video usability information (see Annex E).
>
> X X X X X X
> O O O ...
> X X X X X X
>
> X X X X X X
> O O O
> X X X X X X
>
> X X X X X X
> O O O
> X X X X X X
> . .
> : ´.
> X Location of luma sample
> O Location of chroma sample
>
> Figure 6-1 – Nominal vertical and horizontal locations of 4:2:0 luma and chroma samples in a picture
You are right - I was remembering the special case for BT2020 ("When
chroma_format_idc is equal to 1 (4:2:0 chroma format) and the decoded
video content is intended for interpretation according to Rec. ITU-R
BT.2020-2 or Rec. ITU-R BT.2100-2, chroma_loc_info_present_flag should
be equal to 1, and chroma_sample_loc_type_top_field and
chroma_sample_loc_type_bottom_field should both be equal to 2")
>> 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?
>
>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.
As an aside, with SWS_FAST_BILINEAR (and probably the other methods) in
ffmpeg you need flags=out_v_chr_pos=0:out_h_chr_pos=128 to land the YUV
chroma sample on the top-left RGB sample - that confused me for a while
whilst I was trying to work out what ffmpeg actually does!
Regards
JC
>[...]
>
>thx
More information about the ffmpeg-devel
mailing list