[{"id":34039,"web_url":"https://patchwork.libcamera.org/comment/34039/","msgid":"<174558284863.1586992.14527552509714923515@ping.linuxembedded.co.uk>","date":"2025-04-25T12:07:28","subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-04-25 11:47:03)\n> With the newly introduced `SDLTexture1Plane` it is easy to handle\n> any single-plane format that has an SDL equivalent. So use it for\n> more YUV and RGB formats.\n> \n> The mapping of RGB formats is not entirely straightforward because\n> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of\n> the components is endian dependent, while libcamera's `ZZZ...888...`\n> formats are derived from the matching DRM formats, and the RGB formats\n> in question are defined to be little-endian there. So the\n> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used.\n> \n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++\n> 1 file changed, 36 insertions(+)\n> \n> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp\n> index b295675dc..2edbb523d 100644\n> --- a/src/apps/cam/sdl_sink.cpp\n> +++ b/src/apps/cam/sdl_sink.cpp\n> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)\n> case libcamera::formats::YUYV:\n> texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride);\n> break;\n> + case libcamera::formats::UYVY:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride);\n> + break;\n> + case libcamera::formats::YVYU:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride);\n> + break;\n> + case libcamera::formats::ARGB8888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride);\n> + break;\n> + case libcamera::formats::XRGB8888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride);\n> + break;\n> + case libcamera::formats::RGBA8888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride);\n> + break;\n> + case libcamera::formats::RGBX8888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride);\n> + break;\n> + case libcamera::formats::ABGR8888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride);\n> + break;\n> + case libcamera::formats::XBGR8888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride);\n> + break;\n> + case libcamera::formats::BGRA8888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride);\n> + break;\n> + case libcamera::formats::BGRX8888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride);\n> + break;\n> + case libcamera::formats::RGB888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride);\n> + break;\n> + case libcamera::formats::BGR888:\n> + texture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride);\n> + break;\n\nall of those are in the ordering I would expect:\n\nReviewed-by: Kieran Bingham \n\n> default:\n> std::cerr << \"Unsupported pixel format \"\n> << cfg.pixelFormat.toString() << std::endl;\n> -- \n> 2.49.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 46656BE08B\n\tfor ;\n\tFri, 25 Apr 2025 12:07:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D70E068ACE;\n\tFri, 25 Apr 2025 14:07:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2156A617E5\n\tfor ;\n\tFri, 25 Apr 2025 14:07:31 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 42B2D82E;\n\tFri, 25 Apr 2025 14:07:28 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZO7mDwqQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745582848;\n\tbh=FkIFSEBrl5QgG7bHTHUHiapxAd+BRzFqTX3l9/jUSMw=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=ZO7mDwqQ+Y57EGYLHuSnMVnbAyhBFbS8rgex93MGCE+AedJUo0xuwCSOFASCjMfA3\n\tNpe8jNjEB4ni55tdc1zSml+PMOQvsj2Mex+Ym2Mlxbv5AN9RsxIWGdywC/0wCWKAck\n\trbblqdDbOWtKALaBDdS80QggufplJKAF/eMK+Wyc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>","References":"<20250425104703.805170-1-barnabas.pocze@ideasonboard.com>\n\t<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 25 Apr 2025 13:07:28 +0100","Message-ID":"<174558284863.1586992.14527552509714923515@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34042,"web_url":"https://patchwork.libcamera.org/comment/34042/","msgid":"<20250425124549.GD24730@pendragon.ideasonboard.com>","date":"2025-04-25T12:45:49","subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote:\n> With the newly introduced `SDLTexture1Plane` it is easy to handle\n> any single-plane format that has an SDL equivalent. So use it for\n> more YUV and RGB formats.\n> \n> The mapping of RGB formats is not entirely straightforward because\n> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of\n> the components is endian dependent, while libcamera's `ZZZ...888...`\n> formats are derived from the matching DRM formats, and the RGB formats\n> in question are defined to be little-endian there. So the\n> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used.\n\nFormat mapping is always painful :-(\n\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++\n> 1 file changed, 36 insertions(+)\n> \n> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp\n> index b295675dc..2edbb523d 100644\n> --- a/src/apps/cam/sdl_sink.cpp\n> +++ b/src/apps/cam/sdl_sink.cpp\n> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)\n> \tcase libcamera::formats::YUYV:\n> \t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride);\n> \t\tbreak;\n> +\tcase libcamera::formats::UYVY:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::YVYU:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::ARGB8888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::XRGB8888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::RGBA8888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::RGBX8888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::ABGR8888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::XBGR8888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::BGRA8888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::BGRX8888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::RGB888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride);\n> +\t\tbreak;\n> +\tcase libcamera::formats::BGR888:\n> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride);\n> +\t\tbreak;\n\nI'm tempted to avoid all the constructor calls by instead adding a\nsdlFormat variable and filling it in the switch, and then creating the\ntexture after the switch.\n\nReviewed-by: Laurent Pinchart \n\n> \tdefault:\n> \t\tstd::cerr << \"Unsupported pixel format \"\n> \t\t\t << cfg.pixelFormat.toString() << std::endl;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D3C55BE08B\n\tfor ;\n\tFri, 25 Apr 2025 12:45:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C62A268ACE;\n\tFri, 25 Apr 2025 14:45:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A6BAA617E4\n\tfor ;\n\tFri, 25 Apr 2025 14:45:52 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A220496;\n\tFri, 25 Apr 2025 14:45:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kbG/1gnv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745585149;\n\tbh=G0rz4R6Zfla3GUujxGSbcRaSl4nDZ2jAaXgMim34eZg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kbG/1gnv4qiWwiNVg+5FXtmLo5dP3PHkzD9vSj0+HnOI6TkeAj/iJhqLG7rEZGnmg\n\tGeVf4iGvZ/f8EvHbHcd14EcEszDIdTzzeyw4tIplvQGZ44+g7NR+VHroXExfTL/pIL\n\tFZDWWT+edokMrqSPOImpWm4vftbToP2i14AfiizU=","Date":"Fri, 25 Apr 2025 15:45:49 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","Message-ID":"<20250425124549.GD24730@pendragon.ideasonboard.com>","References":"<20250425104703.805170-1-barnabas.pocze@ideasonboard.com>\n\t<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34043,"web_url":"https://patchwork.libcamera.org/comment/34043/","msgid":"","date":"2025-04-25T12:58:08","subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 04. 25. 14:45 keltezéssel, Laurent Pinchart írta:\n> Hi Barnabás,\n> \n> Thank you for the patch.\n> \n> On Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote:\n>> With the newly introduced `SDLTexture1Plane` it is easy to handle\n>> any single-plane format that has an SDL equivalent. So use it for\n>> more YUV and RGB formats.\n>>\n>> The mapping of RGB formats is not entirely straightforward because\n>> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of\n>> the components is endian dependent, while libcamera's `ZZZ...888...`\n>> formats are derived from the matching DRM formats, and the RGB formats\n>> in question are defined to be little-endian there. So the\n>> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used.\n> \n> Format mapping is always painful :-(\n> \n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++\n>> 1 file changed, 36 insertions(+)\n>>\n>> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp\n>> index b295675dc..2edbb523d 100644\n>> --- a/src/apps/cam/sdl_sink.cpp\n>> +++ b/src/apps/cam/sdl_sink.cpp\n>> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)\n>> \tcase libcamera::formats::YUYV:\n>> \t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride);\n>> \t\tbreak;\n>> +\tcase libcamera::formats::UYVY:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::YVYU:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::ARGB8888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::XRGB8888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::RGBA8888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::RGBX8888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::ABGR8888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::XBGR8888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::BGRA8888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::BGRX8888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::RGB888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride);\n>> +\t\tbreak;\n>> +\tcase libcamera::formats::BGR888:\n>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride);\n>> +\t\tbreak;\n> \n> I'm tempted to avoid all the constructor calls by instead adding a\n> sdlFormat variable and filling it in the switch, and then creating the\n> texture after the switch.\n\nYes, indeed, and I had thought about that for a bit, but there is also\nSDLTextureNV12 (conditionally), and I didn't really see a good way to\nreconcile the two.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n>> \tdefault:\n>> \t\tstd::cerr << \"Unsupported pixel format \"\n>> \t\t\t << cfg.pixelFormat.toString() << std::endl;\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6A590C327D\n\tfor ;\n\tFri, 25 Apr 2025 12:58:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C8CB68ACD;\n\tFri, 25 Apr 2025 14:58:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9F50A617E4\n\tfor ;\n\tFri, 25 Apr 2025 14:58:12 +0200 (CEST)","from [192.168.33.12] (185.221.143.16.nat.pool.zt.hu\n\t[185.221.143.16])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9F8EB82E;\n\tFri, 25 Apr 2025 14:58:09 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ji8zxjpl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745585889;\n\tbh=5nVfJBN7pH8erAER5NKHibSsE88UMSpS7bUOx82O/a4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Ji8zxjplrub0mH1ZAldED+nXHcqq0PK/sBeU9x7Gua7SFMHDqtFkQuzt7Yo8yriKx\n\tz6ZDEvLKCjfbFxCM5Aci1OQp3vDZek5jrCsEifNAf99yPR+S2tPF8bA4shBrvmaV7R\n\twmfc1iyXzDmvcdM63qDOd3o92xjpllM96zkpnVA0=","Message-ID":"","Date":"Fri, 25 Apr 2025 14:58:08 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250425104703.805170-1-barnabas.pocze@ideasonboard.com>\n\t<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>\n\t<20250425124549.GD24730@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250425124549.GD24730@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34048,"web_url":"https://patchwork.libcamera.org/comment/34048/","msgid":"<20250425144101.GF24730@pendragon.ideasonboard.com>","date":"2025-04-25T14:41:01","subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Apr 25, 2025 at 02:58:08PM +0200, Barnabás Pőcze wrote:\n> Hi\n> \n> \n> 2025. 04. 25. 14:45 keltezéssel, Laurent Pinchart írta:\n> > Hi Barnabás,\n> > \n> > Thank you for the patch.\n> > \n> > On Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote:\n> >> With the newly introduced `SDLTexture1Plane` it is easy to handle\n> >> any single-plane format that has an SDL equivalent. So use it for\n> >> more YUV and RGB formats.\n> >>\n> >> The mapping of RGB formats is not entirely straightforward because\n> >> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of\n> >> the components is endian dependent, while libcamera's `ZZZ...888...`\n> >> formats are derived from the matching DRM formats, and the RGB formats\n> >> in question are defined to be little-endian there. So the\n> >> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used.\n> > \n> > Format mapping is always painful :-(\n> > \n> >> Signed-off-by: Barnabás Pőcze \n> >> ---\n> >> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++\n> >> 1 file changed, 36 insertions(+)\n> >>\n> >> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp\n> >> index b295675dc..2edbb523d 100644\n> >> --- a/src/apps/cam/sdl_sink.cpp\n> >> +++ b/src/apps/cam/sdl_sink.cpp\n> >> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)\n> >> \tcase libcamera::formats::YUYV:\n> >> \t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride);\n> >> \t\tbreak;\n> >> +\tcase libcamera::formats::UYVY:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::YVYU:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::ARGB8888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::XRGB8888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::RGBA8888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::RGBX8888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::ABGR8888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::XBGR8888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::BGRA8888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::BGRX8888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::RGB888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride);\n> >> +\t\tbreak;\n> >> +\tcase libcamera::formats::BGR888:\n> >> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride);\n> >> +\t\tbreak;\n> > \n> > I'm tempted to avoid all the constructor calls by instead adding a\n> > sdlFormat variable and filling it in the switch, and then creating the\n> > texture after the switch.\n> \n> Yes, indeed, and I had thought about that for a bit, but there is also\n> SDLTextureNV12 (conditionally), and I didn't really see a good way to\n> reconcile the two.\n\n\n\tswitch (cfg.pixelFormat) {\n#ifdef HAVE_LIBJPEG\n\tcase libcamera::formats::MJPEG:\n\t\ttexture_ = std::make_unique(rect_);\n\t\tbreak;\n#endif\n#if SDL_VERSION_ATLEAST(2, 0, 16)\n\tcase libcamera::formats::NV12:\n\t\ttexture_ = std::make_unique(rect_, cfg.stride);\n\t\tbreak;\n#endif\n \tcase libcamera::formats::YUYV:\n \t\tsdlFormat = SDL_PIXELFORMAT_YUY2;\n \t\tbreak;\n\tcase libcamera::formats::UYVY:\n \t\tsdlFormat = SDL_PIXELFORMAT_UYVY;\n\t\tbreak;\n...\n\tdefault:\n\t\tstd::cerr << \"Unsupported pixel format \"\n\t\t\t << cfg.pixelFormat.toString() << std::endl;\n\t\treturn -EINVAL;\n\t};\n\n\tif (sdlFormat)\n\t\ttexture_ = std::make_unique(rect_, sdlFormat, cfg.stride);\n\n> > Reviewed-by: Laurent Pinchart \n> > \n> >> \tdefault:\n> >> \t\tstd::cerr << \"Unsupported pixel format \"\n> >> \t\t\t << cfg.pixelFormat.toString() << std::endl;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 12AA8C327D\n\tfor ;\n\tFri, 25 Apr 2025 14:41:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2464F68ACE;\n\tFri, 25 Apr 2025 16:41:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E02CD617E4\n\tfor ;\n\tFri, 25 Apr 2025 16:41:04 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A3C6E82E;\n\tFri, 25 Apr 2025 16:41:01 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gW6HEfkj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745592061;\n\tbh=KcQ2/zVfa2aB1qc/Qr6j6ruuWPMYNyfQg9sfg1g1f7A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gW6HEfkjUExWvLZyHQ2A3lrvkZlcLpJ8sCEptYYsyysPvCOK0hSlc6QKlrU0Kw1SY\n\tDONgBGKBFE6kdrVKRr3PUmMnSvBgyiMhGcuNGPhJFzdGHARtL8tnUVyjFlo4E167J8\n\tsPlBu+j85mIdWAkU73U/yDElbDEX/LFRatm/zR6o=","Date":"Fri, 25 Apr 2025 17:41:01 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","Message-ID":"<20250425144101.GF24730@pendragon.ideasonboard.com>","References":"<20250425104703.805170-1-barnabas.pocze@ideasonboard.com>\n\t<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>\n\t<20250425124549.GD24730@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34066,"web_url":"https://patchwork.libcamera.org/comment/34066/","msgid":"<5679a10c-bdc3-4502-9c58-00d70ff81462@ideasonboard.com>","date":"2025-04-28T08:52:39","subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 04. 25. 16:41 keltezéssel, Laurent Pinchart írta:\n> On Fri, Apr 25, 2025 at 02:58:08PM +0200, Barnabás Pőcze wrote:\n>> Hi\n>>\n>>\n>> 2025. 04. 25. 14:45 keltezéssel, Laurent Pinchart írta:\n>>> Hi Barnabás,\n>>>\n>>> Thank you for the patch.\n>>>\n>>> On Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote:\n>>>> With the newly introduced `SDLTexture1Plane` it is easy to handle\n>>>> any single-plane format that has an SDL equivalent. So use it for\n>>>> more YUV and RGB formats.\n>>>>\n>>>> The mapping of RGB formats is not entirely straightforward because\n>>>> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of\n>>>> the components is endian dependent, while libcamera's `ZZZ...888...`\n>>>> formats are derived from the matching DRM formats, and the RGB formats\n>>>> in question are defined to be little-endian there. So the\n>>>> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used.\n>>>\n>>> Format mapping is always painful :-(\n>>>\n>>>> Signed-off-by: Barnabás Pőcze \n>>>> ---\n>>>> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++\n>>>> 1 file changed, 36 insertions(+)\n>>>>\n>>>> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp\n>>>> index b295675dc..2edbb523d 100644\n>>>> --- a/src/apps/cam/sdl_sink.cpp\n>>>> +++ b/src/apps/cam/sdl_sink.cpp\n>>>> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)\n>>>> \tcase libcamera::formats::YUYV:\n>>>> \t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride);\n>>>> \t\tbreak;\n>>>> +\tcase libcamera::formats::UYVY:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::YVYU:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::ARGB8888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::XRGB8888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::RGBA8888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::RGBX8888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::ABGR8888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::XBGR8888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::BGRA8888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::BGRX8888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::RGB888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride);\n>>>> +\t\tbreak;\n>>>> +\tcase libcamera::formats::BGR888:\n>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride);\n>>>> +\t\tbreak;\n>>>\n>>> I'm tempted to avoid all the constructor calls by instead adding a\n>>> sdlFormat variable and filling it in the switch, and then creating the\n>>> texture after the switch.\n>>\n>> Yes, indeed, and I had thought about that for a bit, but there is also\n>> SDLTextureNV12 (conditionally), and I didn't really see a good way to\n>> reconcile the two.\n> \n> \n> \tswitch (cfg.pixelFormat) {\n> #ifdef HAVE_LIBJPEG\n> \tcase libcamera::formats::MJPEG:\n> \t\ttexture_ = std::make_unique(rect_);\n> \t\tbreak;\n> #endif\n> #if SDL_VERSION_ATLEAST(2, 0, 16)\n> \tcase libcamera::formats::NV12:\n> \t\ttexture_ = std::make_unique(rect_, cfg.stride);\n> \t\tbreak;\n> #endif\n> \tcase libcamera::formats::YUYV:\n> \t\tsdlFormat = SDL_PIXELFORMAT_YUY2;\n> \t\tbreak;\n> \tcase libcamera::formats::UYVY:\n> \t\tsdlFormat = SDL_PIXELFORMAT_UYVY;\n> \t\tbreak;\n> ...\n> \tdefault:\n> \t\tstd::cerr << \"Unsupported pixel format \"\n> \t\t\t << cfg.pixelFormat.toString() << std::endl;\n> \t\treturn -EINVAL;\n> \t};\n> \n> \tif (sdlFormat)\n> \t\ttexture_ = std::make_unique(rect_, sdlFormat, cfg.stride);\n> \n\nI have looked at something like this, but I am not a fan at all. :(\n\n\nRegards,\nBarnabás Pőcze\n\n\n>>> Reviewed-by: Laurent Pinchart \n>>>\n>>>> \tdefault:\n>>>> \t\tstd::cerr << \"Unsupported pixel format \"\n>>>> \t\t\t << cfg.pixelFormat.toString() << std::endl;\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7BBD3BE08B\n\tfor ;\n\tMon, 28 Apr 2025 08:52:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B280A68ACF;\n\tMon, 28 Apr 2025 10:52:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7C1868ACA\n\tfor ;\n\tMon, 28 Apr 2025 10:52:42 +0200 (CEST)","from [192.168.33.12] (185.221.141.190.nat.pool.zt.hu\n\t[185.221.141.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E3A2B7E6;\n\tMon, 28 Apr 2025 10:52:37 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Rz9d47BB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745830358;\n\tbh=ZRrQDg1/xbeCY7i5nqSXHig3Yu9X56Pq9/ZsMvsJ8oc=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Rz9d47BBeqM+jIsq+gcSGmJJJXug/DxzwUeAqH+UKmWscGNX4g5CyLqRt4102B/94\n\t02h2s/obPP0DbD7g+EhoYOe5T7HcftxZAxmDeaCyg30HZ7f0XRr1MkRXi0vxC6QiBt\n\tTlJNhTOKfx5edRw1TRGar901s+ltYkB8qX/s+IEs=","Message-ID":"<5679a10c-bdc3-4502-9c58-00d70ff81462@ideasonboard.com>","Date":"Mon, 28 Apr 2025 10:52:39 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250425104703.805170-1-barnabas.pocze@ideasonboard.com>\n\t<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>\n\t<20250425124549.GD24730@pendragon.ideasonboard.com>\n\t\n\t<20250425144101.GF24730@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250425144101.GF24730@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34067,"web_url":"https://patchwork.libcamera.org/comment/34067/","msgid":"<20250428091415.GA3371@pendragon.ideasonboard.com>","date":"2025-04-28T09:14:15","subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Apr 28, 2025 at 10:52:39AM +0200, Barnabás Pőcze wrote:\n> 2025. 04. 25. 16:41 keltezéssel, Laurent Pinchart írta:\n> > On Fri, Apr 25, 2025 at 02:58:08PM +0200, Barnabás Pőcze wrote:\n> >> 2025. 04. 25. 14:45 keltezéssel, Laurent Pinchart írta:\n> >>> On Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote:\n> >>>> With the newly introduced `SDLTexture1Plane` it is easy to handle\n> >>>> any single-plane format that has an SDL equivalent. So use it for\n> >>>> more YUV and RGB formats.\n> >>>>\n> >>>> The mapping of RGB formats is not entirely straightforward because\n> >>>> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of\n> >>>> the components is endian dependent, while libcamera's `ZZZ...888...`\n> >>>> formats are derived from the matching DRM formats, and the RGB formats\n> >>>> in question are defined to be little-endian there. So the\n> >>>> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used.\n> >>>\n> >>> Format mapping is always painful :-(\n> >>>\n> >>>> Signed-off-by: Barnabás Pőcze \n> >>>> ---\n> >>>> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++\n> >>>> 1 file changed, 36 insertions(+)\n> >>>>\n> >>>> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp\n> >>>> index b295675dc..2edbb523d 100644\n> >>>> --- a/src/apps/cam/sdl_sink.cpp\n> >>>> +++ b/src/apps/cam/sdl_sink.cpp\n> >>>> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)\n> >>>> \tcase libcamera::formats::YUYV:\n> >>>> \t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride);\n> >>>> \t\tbreak;\n> >>>> +\tcase libcamera::formats::UYVY:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::YVYU:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::ARGB8888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::XRGB8888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::RGBA8888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::RGBX8888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::ABGR8888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::XBGR8888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::BGRA8888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::BGRX8888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::RGB888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>> +\tcase libcamera::formats::BGR888:\n> >>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride);\n> >>>> +\t\tbreak;\n> >>>\n> >>> I'm tempted to avoid all the constructor calls by instead adding a\n> >>> sdlFormat variable and filling it in the switch, and then creating the\n> >>> texture after the switch.\n> >>\n> >> Yes, indeed, and I had thought about that for a bit, but there is also\n> >> SDLTextureNV12 (conditionally), and I didn't really see a good way to\n> >> reconcile the two.\n> > \n> > \n> > \tswitch (cfg.pixelFormat) {\n> > #ifdef HAVE_LIBJPEG\n> > \tcase libcamera::formats::MJPEG:\n> > \t\ttexture_ = std::make_unique(rect_);\n> > \t\tbreak;\n> > #endif\n> > #if SDL_VERSION_ATLEAST(2, 0, 16)\n> > \tcase libcamera::formats::NV12:\n> > \t\ttexture_ = std::make_unique(rect_, cfg.stride);\n> > \t\tbreak;\n> > #endif\n> > \tcase libcamera::formats::YUYV:\n> > \t\tsdlFormat = SDL_PIXELFORMAT_YUY2;\n> > \t\tbreak;\n> > \tcase libcamera::formats::UYVY:\n> > \t\tsdlFormat = SDL_PIXELFORMAT_UYVY;\n> > \t\tbreak;\n> > ...\n> > \tdefault:\n> > \t\tstd::cerr << \"Unsupported pixel format \"\n> > \t\t\t << cfg.pixelFormat.toString() << std::endl;\n> > \t\treturn -EINVAL;\n> > \t};\n> > \n> > \tif (sdlFormat)\n> > \t\ttexture_ = std::make_unique(rect_, sdlFormat, cfg.stride);\n> > \n> \n> I have looked at something like this, but I am not a fan at all. :(\n\nI understand why and I probably have the same feeling. I still think\nit's a bit better than duplicating the calls though.\n\n> >>> Reviewed-by: Laurent Pinchart \n> >>>\n> >>>> \tdefault:\n> >>>> \t\tstd::cerr << \"Unsupported pixel format \"\n> >>>> \t\t\t << cfg.pixelFormat.toString() << std::endl;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6E507BE08B\n\tfor ;\n\tMon, 28 Apr 2025 09:14:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E33868AD4;\n\tMon, 28 Apr 2025 11:14:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3147068ACF\n\tfor ;\n\tMon, 28 Apr 2025 11:14:22 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E5AC7475;\n\tMon, 28 Apr 2025 11:14:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"H1FZ1VLf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745831657;\n\tbh=u8gpszcFHSsdvXGBja+HC1A+RFYO2atibONR7ETDK/U=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H1FZ1VLfiXSuVXRuRoTHzgQG11ePJKsmF4K6cCozzHEtoWoybmqpjiiSTrVkwN72U\n\tk7VWVk/r1C0IF0SQKRH/kbH5pGbIthw3vc2Z8xSZoSu8KP11/fP4W6JIJg6IWMB4/S\n\tJgZi7q+1ABT+7JjI22F4sLxkdhZcuG5jKA6J2ysI=","Date":"Mon, 28 Apr 2025 12:14:15 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","Message-ID":"<20250428091415.GA3371@pendragon.ideasonboard.com>","References":"<20250425104703.805170-1-barnabas.pocze@ideasonboard.com>\n\t<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>\n\t<20250425124549.GD24730@pendragon.ideasonboard.com>\n\t\n\t<20250425144101.GF24730@pendragon.ideasonboard.com>\n\t<5679a10c-bdc3-4502-9c58-00d70ff81462@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<5679a10c-bdc3-4502-9c58-00d70ff81462@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34082,"web_url":"https://patchwork.libcamera.org/comment/34082/","msgid":"<13308887-2795-4472-889a-5685c2351dbe@ideasonboard.com>","date":"2025-04-30T07:59:24","subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 04. 28. 11:14 keltezéssel, Laurent Pinchart írta:\n> On Mon, Apr 28, 2025 at 10:52:39AM +0200, Barnabás Pőcze wrote:\n>> 2025. 04. 25. 16:41 keltezéssel, Laurent Pinchart írta:\n>>> On Fri, Apr 25, 2025 at 02:58:08PM +0200, Barnabás Pőcze wrote:\n>>>> 2025. 04. 25. 14:45 keltezéssel, Laurent Pinchart írta:\n>>>>> On Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote:\n>>>>>> With the newly introduced `SDLTexture1Plane` it is easy to handle\n>>>>>> any single-plane format that has an SDL equivalent. So use it for\n>>>>>> more YUV and RGB formats.\n>>>>>>\n>>>>>> The mapping of RGB formats is not entirely straightforward because\n>>>>>> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of\n>>>>>> the components is endian dependent, while libcamera's `ZZZ...888...`\n>>>>>> formats are derived from the matching DRM formats, and the RGB formats\n>>>>>> in question are defined to be little-endian there. So the\n>>>>>> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used.\n>>>>>\n>>>>> Format mapping is always painful :-(\n>>>>>\n>>>>>> Signed-off-by: Barnabás Pőcze \n>>>>>> ---\n>>>>>> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++\n>>>>>> 1 file changed, 36 insertions(+)\n>>>>>>\n>>>>>> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp\n>>>>>> index b295675dc..2edbb523d 100644\n>>>>>> --- a/src/apps/cam/sdl_sink.cpp\n>>>>>> +++ b/src/apps/cam/sdl_sink.cpp\n>>>>>> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)\n>>>>>> \tcase libcamera::formats::YUYV:\n>>>>>> \t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride);\n>>>>>> \t\tbreak;\n>>>>>> +\tcase libcamera::formats::UYVY:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::YVYU:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::ARGB8888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::XRGB8888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::RGBA8888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::RGBX8888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::ABGR8888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::XBGR8888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::BGRA8888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::BGRX8888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::RGB888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>> +\tcase libcamera::formats::BGR888:\n>>>>>> +\t\ttexture_ = std::make_unique(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride);\n>>>>>> +\t\tbreak;\n>>>>>\n>>>>> I'm tempted to avoid all the constructor calls by instead adding a\n>>>>> sdlFormat variable and filling it in the switch, and then creating the\n>>>>> texture after the switch.\n>>>>\n>>>> Yes, indeed, and I had thought about that for a bit, but there is also\n>>>> SDLTextureNV12 (conditionally), and I didn't really see a good way to\n>>>> reconcile the two.\n>>>\n>>>\n>>> \tswitch (cfg.pixelFormat) {\n>>> #ifdef HAVE_LIBJPEG\n>>> \tcase libcamera::formats::MJPEG:\n>>> \t\ttexture_ = std::make_unique(rect_);\n>>> \t\tbreak;\n>>> #endif\n>>> #if SDL_VERSION_ATLEAST(2, 0, 16)\n>>> \tcase libcamera::formats::NV12:\n>>> \t\ttexture_ = std::make_unique(rect_, cfg.stride);\n>>> \t\tbreak;\n>>> #endif\n>>> \tcase libcamera::formats::YUYV:\n>>> \t\tsdlFormat = SDL_PIXELFORMAT_YUY2;\n>>> \t\tbreak;\n>>> \tcase libcamera::formats::UYVY:\n>>> \t\tsdlFormat = SDL_PIXELFORMAT_UYVY;\n>>> \t\tbreak;\n>>> ...\n>>> \tdefault:\n>>> \t\tstd::cerr << \"Unsupported pixel format \"\n>>> \t\t\t << cfg.pixelFormat.toString() << std::endl;\n>>> \t\treturn -EINVAL;\n>>> \t};\n>>>\n>>> \tif (sdlFormat)\n>>> \t\ttexture_ = std::make_unique(rect_, sdlFormat, cfg.stride);\n>>>\n>>\n>> I have looked at something like this, but I am not a fan at all. :(\n> \n> I understand why and I probably have the same feeling. I still think\n> it's a bit better than duplicating the calls though.\n\nPlease see the latest version: https://patchwork.libcamera.org/patch/23305/\n\n\n> \n>>>>> Reviewed-by: Laurent Pinchart \n>>>>>\n>>>>>> \tdefault:\n>>>>>> \t\tstd::cerr << \"Unsupported pixel format \"\n>>>>>> \t\t\t << cfg.pixelFormat.toString() << std::endl;\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1566FC327D\n\tfor ;\n\tWed, 30 Apr 2025 07:59:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C28A968AD8;\n\tWed, 30 Apr 2025 09:59:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD6E3617DE\n\tfor ;\n\tWed, 30 Apr 2025 09:59:27 +0200 (CEST)","from [192.168.33.12] (185.221.141.190.nat.pool.zt.hu\n\t[185.221.141.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8DC9D725;\n\tWed, 30 Apr 2025 09:59:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fW+aCV1L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745999961;\n\tbh=/Q9IQkE1SaSzI5f7PcLvlhdAhPC5WR7gk6HGtnC+1Vs=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=fW+aCV1L2MwfPna8K6AWg31RFi/W4beg89kpxR8nXOvxFlnRuXVSUO5ITQheTgOSO\n\tt7wwd8J4NzGH38dJcOvba9XEUP2vqjFre6xVh6ZUlXkduzInajrOoRXq4RTyqAHuwH\n\tBcMb4TBKaxIdYMUHdFTGMLczsgG7odXGIV66pPo0=","Message-ID":"<13308887-2795-4472-889a-5685c2351dbe@ideasonboard.com>","Date":"Wed, 30 Apr 2025 09:59:24 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 4/4] apps: cam: sdl_sink: Support more single-plane\n\tformats","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250425104703.805170-1-barnabas.pocze@ideasonboard.com>\n\t<20250425104703.805170-5-barnabas.pocze@ideasonboard.com>\n\t<20250425124549.GD24730@pendragon.ideasonboard.com>\n\t\n\t<20250425144101.GF24730@pendragon.ideasonboard.com>\n\t<5679a10c-bdc3-4502-9c58-00d70ff81462@ideasonboard.com>\n\t<20250428091415.GA3371@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250428091415.GA3371@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]