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 3623C46EA1 for ; Sun, 20 Aug 2023 18:09:45 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E546468BD48; Sun, 20 Aug 2023 21:09:42 +0300 (EEST) Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9FB89680897 for ; Sun, 20 Aug 2023 21:09:36 +0300 (EEST) Received: by mail-wm1-f50.google.com with SMTP id 5b1f17b1804b1-3fe9c20f449so24833355e9.3 for ; Sun, 20 Aug 2023 11:09:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kynesim-co-uk.20221208.gappssmtp.com; s=20221208; t=1692554976; x=1693159776; h=content-transfer-encoding:mime-version:user-agent:in-reply-to :references:message-id:date:subject:to:from:from:to:cc:subject:date :message-id:reply-to; bh=m5Om5Xpfczx0t7nW6+1HSDP8TtsP8LUR0nAvfsE3fPY=; b=1LWIXFWEih4jqHzHKOJ3mP4wUz3oDR4PvTEFtL6SRpSOVr2As3yV0/87WmAt1pmLYF 60Zb3HQahycqNx7nC8a+RLTsclwjKMWBdriouFT8W1BbSKTJ29AD2Uat3MCnr7i3Un9O 5Gm4b6foFwoTQJT7Dxwu9kxZm4mv0W75jagr6COqF0YHPOyOeRzyjSqqpQNyAonvqbM1 Cq1WTDBha9tBbow64azyOI3o4YdSvVtSjuW10z4Bm08yXxQ0j1bAzRDPIjY/QeTlZYEn 76tdO/NAOB5+n36QlxT4dp6lxwhGV+4B9yt92ztdmYw/6g10zWKslvs05NBGKweeuwD0 Pibg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692554976; x=1693159776; h=content-transfer-encoding:mime-version:user-agent:in-reply-to :references:message-id:date:subject:to:from:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=m5Om5Xpfczx0t7nW6+1HSDP8TtsP8LUR0nAvfsE3fPY=; b=b2VtbQ62ZvBIrhCgcFhw/fmaJI0qy0Qqg49cSeIH11gNdQOTZTIaQqN7SrEuhxG08m f8pRSTuJkToPljPT5vPZDnylTFQgzXFZLKaSzSCz+DbCg8n1dqb9oMvddaook8yQO5ZQ m7ctWaKH3hhyJp93yJFFtAyygVHDocWbd0eBl4K0HafcP4u0h+FE1J2vhw1zwAuZ4mkX 0I7igTImqnvQXvsh18vkX6RdyouISXjqWyoWHRt7PILZBR8zySUIUXimhXiuKbM04mbR orglZ3C63O7MH0WigQqYgfDi/bJdPAGLhq0O6vXvQyKjzzkvcJpAtmWWeGdLprq/+8hB TWoQ== X-Gm-Message-State: AOJu0YwaQ/ocClCaiai+0UoLno2yJzs4n+VIptNVPXZ8sOaUl0pQ9Gjb Mq+cwOOb6sQ6hdio+9v6cYAfKDZDa0c/S/pGnBY= X-Google-Smtp-Source: AGHT+IE48Mt1AGdNvpMy8m5WewxGRyvnQ2zAgmHkgfdKC/8HLHTeFU7qDSPbdYE0Tb3X7xvknJNwCw== X-Received: by 2002:a1c:4c15:0:b0:3fb:dd5d:76b with SMTP id z21-20020a1c4c15000000b003fbdd5d076bmr3210677wmf.7.1692554975693; Sun, 20 Aug 2023 11:09:35 -0700 (PDT) Received: from CTHALPA.outer.uphall.net (cpc1-cmbg20-2-0-cust759.5-4.cable.virginm.net. [86.21.218.248]) by smtp.gmail.com with ESMTPSA id l11-20020a1c790b000000b003fbca942499sm13274502wme.14.2023.08.20.11.09.35 for (version=TLS1 cipher=ECDHE-ECDSA-AES128-SHA bits=128/128); Sun, 20 Aug 2023 11:09:35 -0700 (PDT) From: John Cox To: FFmpeg development discussions and patches Date: Sun, 20 Aug 2023 19:09:34 +0100 Message-ID: References: <20230820151022.2204421-1-jc@kynesim.co.uk> <20230820151022.2204421-4-jc@kynesim.co.uk> <20230820171614.GC7802@pb2> In-Reply-To: <20230820171614.GC7802@pb2> User-Agent: ForteAgent/8.00.32.1272 MIME-Version: 1.0 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: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Sun, 20 Aug 2023 19:16:14 +0200, you 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 >> --- >> 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 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 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, const uint8_t *src[], >> int srcStride[], int srcSliceY, int srcSliceH, >> uint8_t *dst[], int dstStride[]) > >> @@ -2035,8 +2052,13 @@ void ff_get_unscaled_swscale(SwsContext *c) >> /* bgr24toYV12 */ >> if (srcFormat == AV_PIX_FMT_BGR24 && >> (dstFormat == AV_PIX_FMT_YUV420P || dstFormat == AV_PIX_FMT_YUVA420P) && >> - !(flags & SWS_ACCURATE_RND) && !(dstW&1)) >> + !(flags & (SWS_ACCURATE_RND | SWS_BITEXACT)) && !(dstW&1)) > >this doesnt belong in this patch So should it go in its own patch, or attached to some other patch? Ta _______________________________________________ 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".