[EXT] Re: [PATCH] gstreamer: Add Y444 format support to the YUV list

[Kieran Bingham]

Quoting Antoine Bouyer (2025-03-18 10:56:08)
> Hi Kieran,
> Thanks for your review.
> 
> On 17/03/2025 18:52, Kieran Bingham wrote:
> > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
> > 
> > 
> > Quoting Antoine Bouyer (2025-03-17 16:10:50)
> >> 'imx8-isi' pipeline provides support for 'YUV444' PixelFormat with YUV
> >> streams, but it cannot be played with gstreamer adapter whereas
> >> gstreamer's video format 'Y444' value suggests that it also supports
> >> this format.
> > 
> > 'Suggests'? I think we can be a bit more confident in the wording that
> > we expect Y444 from gstreamer to be mapped to the YUV444 from libcamera.
> 
> Should I submit new version of the patch and rephrase commit message 
> accordingly ?

No need at the moment, I'll await a bit more time for more review (ahem,
mostly because gitlab.freedesktop.org is down this week for migration)
so I hope this will be collected next week, before the next libcamera
release. There's nothing complex here that I would worry about for
merging.

If you have a more confident wording proposal, we can update the text on
applying but it doesn't matter too much.

> > I assume you have tested this as well, so we can 'confirm' that this is
> > indeed the correct format mappings ?
> > 
> 
> Yes I did. Colors are as expected.

Great! Thanks for confirming.

--
Kieran

> 
> >>
> >> To add support of Planar 4:4:4 YUV format in gstreamer adapter, this patch
> >> maps 'Y444' gstreamer video format with 'YUV444' libcamera PixelFormat.
> >>
> >> Then below command example can be used to capture a stream with imx8-isi
> >> pipeline:
> >>
> >>    gst-launch-1.0 \
> >>        libcamerasrc camera-name=<your_camera_name> ! \
> >>        video/x-raw, format=Y444, width=1280, height=800 ! \
> >>        queue ! \
> >>        filesink location=/tmp/output
> > 
> > Not always something we would have in the commit message, but I think
> > clearly stating how this is tested here is useful!
> > 
> >>
> >> Signed-off-by: Antoine Bouyer <antoine.bouyer at nxp.com>
> > 
> > Overall, that's an easy one :-)
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> >> ---
> >>   src/gstreamer/gstlibcamera-utils.cpp | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> >> index a466b30..41eea7d 100644
> >> --- a/src/gstreamer/gstlibcamera-utils.cpp
> >> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> >> @@ -74,6 +74,7 @@ static struct {
> >>          { GST_VIDEO_FORMAT_I420, formats::YUV420 },
> >>          { GST_VIDEO_FORMAT_YV12, formats::YVU420 },
> >>          { GST_VIDEO_FORMAT_Y42B, formats::YUV422 },
> >> +       { GST_VIDEO_FORMAT_Y444, formats::YUV444 },
> >>
> >>          /* YUV Packed */
> >>          { GST_VIDEO_FORMAT_UYVY, formats::UYVY },
> >> --
> >> 2.34.1
> >>