From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 1211045326 for ; Mon, 21 Aug 2023 19:15:49 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6591868C279; Mon, 21 Aug 2023 22:15:46 +0300 (EEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B3E9F6802C0 for ; Mon, 21 Aug 2023 22:15:39 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id C3514240003 for ; Mon, 21 Aug 2023 19:15:38 +0000 (UTC) Date: Mon, 21 Aug 2023 21:15:37 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20230821191537.GE7802@pb2> References: <20230820151022.2204421-1-jc@kynesim.co.uk> <20230820151022.2204421-4-jc@kynesim.co.uk> <20230820171614.GC7802@pb2> <20230820174511.GD7802@pb2> <5pl4eilterjun2l0up51r8jg1pvdluq7b2@4ax.com> MIME-Version: 1.0 In-Reply-To: <5pl4eilterjun2l0up51r8jg1pvdluq7b2@4ax.com> X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH v1 3/6] swscale: Add explicit rgb24->yv12 conversion X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: multipart/mixed; boundary="===============8635695194773885835==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============8635695194773885835== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="5El/nexOlxEtImhD" Content-Disposition: inline --5El/nexOlxEtImhD Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Aug 20, 2023 at 07:28:40PM +0100, John Cox wrote: > On Sun, 20 Aug 2023 19:45:11 +0200, you wrote: >=20 > >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. > >> >=20 > >> > 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(-) > >> >=20 > >> > 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 srcStrid= e, > >> > 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 srcStrid= e, > >> > + int32_t *rgb2yuv); > >> > void (*planar2x)(const uint8_t *src, uint8_t *dst, int width, int h= eight, > >> > int srcStride, int dstStride); > >> > void (*interleaveBytes)(const uint8_t *src1, const uint8_t *src2, u= int8_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 *u= dst, > >> > uint8_t *vdst, int width, int height, int lum= Stride, > >> > int chromStride, int srcStride, int32_t *rgb2= yuv); > >> > +void ff_rgb24toyv12_c(const uint8_t *src, uint8_t *ydst, uint8_t *u= dst, > >> > + uint8_t *vdst, int width, int height, int lum= Stride, > >> > + int chromStride, int srcStride, int32_t *rgb2= yuv); > >> > =20 > >> > /** > >> > * 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 *sr= c, uint8_t *ydst, uint8_t *udst, > >> > int width, int height, > >> > int lumStride, int chromStride, int s= rcStride, > >> > int32_t *rgb2yuv); > >> > +extern void (*ff_rgb24toyv12)(const uint8_t *src, uint8_t *ydst, ui= nt8_t *udst, uint8_t *vdst, > >> > + int width, int height, > >> > + int lumStride, int chromStride, int s= rcStride, > >> > + int32_t *rgb2yuv); > >> > extern void (*planar2x)(const uint8_t *src, uint8_t *dst, int width= , int height, > >> > int srcStride, int dstStride); > >> > =20 > >> > diff --git a/libswscale/rgb2rgb_template.c b/libswscale/rgb2rgb_temp= late.c > >> > index 8ef4a2cf5d..e57bfa6545 100644 > >> > --- a/libswscale/rgb2rgb_template.c > >> > +++ b/libswscale/rgb2rgb_template.c > >>=20 > >>=20 > >> > @@ -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 *u= dst, > >> > +static void rgb24toyv12_x(const uint8_t *src, uint8_t *ydst, uint8_= t *udst, > >>=20 > >> this probably should be inline > >>=20 > >> also i see now "FIXME: Write HQ version." above here. Do you really wa= nt to > >> add a low quality rgb24toyv12 ? > >> (it is vissible on the diagonal border (cyan / red )) in > >> ./ffmpeg -f lavfi -i testsrc=3Dsize=3D5632x3168 -pix_fmt yuv420p -vfr= ames 1 -qscale 1 -strict -1 new.jpg > >>=20 > >> 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 >=20 > I thought it was a bit light but it is what the existing code did >=20 > >2. the chroma sample locations are ignored, the locations for yuv420 are= reaonable standard >=20 > 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 . . : =C2=B4. X Location of luma sample O Location of chroma sample Figure 6-1 =E2=80=93 Nominal vertical and horizontal locations of 4:2:0= luma and chroma samples in a picture > 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? >=20 > >this needs some simple filter to get from a few RGB samples to the RGB s= ample co-located > >with ths UV sample before RGB->UV >=20 > 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 [...] thx --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The day soldiers stop bringing you their problems is the day you have stopp= ed=20 leading them. They have either lost confidence that you can help or conclud= ed=20 you do not care. Either case is a failure of leadership. - Colin Powell --5El/nexOlxEtImhD Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZOO30wAKCRBhHseHBAsP q782AJ9AzWZ8RU+b/hE02kcRwK7ivnP+hwCbBOdVCuclSQXnflt0aO7XUoYf108= =B+Z9 -----END PGP SIGNATURE----- --5El/nexOlxEtImhD-- --===============8635695194773885835== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============8635695194773885835==--