| Message ID | 20250317161050.2008382-1-antoine.bouyer@nxp.com |
|---|---|
| State | Accepted |
| Commit | 06269e9584efa42b359bbeef3240dc19ffd4e261 |
| Headers | show |
| Series |
|
| Related | show |
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. I assume you have tested this as well, so we can 'confirm' that this is indeed the correct format mappings ? > > 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@nxp.com> Overall, that's an easy one :-) Reviewed-by: Kieran Bingham <kieran.bingham@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 >
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 ? > > 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. >> >> 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@nxp.com> > > Overall, that's an easy one :-) > > Reviewed-by: Kieran Bingham <kieran.bingham@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 >>
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@nxp.com> > > > > Overall, that's an easy one :-) > > > > Reviewed-by: Kieran Bingham <kieran.bingham@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 > >>
Le lundi 17 mars 2025 à 17:10 +0100, Antoine Bouyer a écrit : > '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. > > 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 > > Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.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 }, Thanks. Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > /* YUV Packed */ > { GST_VIDEO_FORMAT_UYVY, formats::UYVY },
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 },
'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. 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 Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com> --- src/gstreamer/gstlibcamera-utils.cpp | 1 + 1 file changed, 1 insertion(+)