[PATCH v3] gstreamer: Implement caps negotiation for video/x-bayer

Kieran Bingham kieran.bingham at ideasonboard.com
Mon Nov 4 16:12:51 CET 2024


Quoting Kieran Bingham (2024-11-04 15:02:28)
> Quoting Nicolas Dufresne (2024-11-04 14:58:01)
> > Thanks.
> > 
> > Le lundi 04 novembre 2024 à 12:36 +0100, mailinglist1 at johanneskirchmair.de a
> > écrit :
> > > From: Johannes Kirchmair <johannes.kirchmair at skidata.com>
> > > 
> > > The parsing of video/x-bayer sources from string makes it possible to
> > > use cameras providing e.g SGRBG8 streams via gst-launch.
> > > 
> > > Like:
> > > gst-launch-1.0 libcamerasrc camera-name=<cam> ! video/x-bayer,format=grbg
> > > 
> > > Without this change the gstreamer plugin complains about "Unsupported
> > > media type: video/x-bayer".
> > > 
> > > Signed-off-by: Johannes Kirchmair <johannes.kirchmair at skidata.com>
> > 
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> I'll merge this assuming the CI tests all pass cleanly.

https://gitlab.freedesktop.org/camera/libcamera/-/jobs/66072263 reports:

```
5c8010e4f8dc2d4e41ce817c03289b1155ccd659 gstreamer: Implement caps negotiation for video/x-bayer
------------------------------------------------------------------------------------------------
No 'Signed-off-by' trailer matching author 'Johannes Kirchmair <johannes.kirchmair at skidata.com>', see Documentation/contributing.rst
--- src/gstreamer/gstlibcamera-utils.cpp
+++ src/gstreamer/gstlibcamera-utils.cpp
@@ -283,7 +283,7 @@
 static const gchar *
 bayer_format_to_string(PixelFormat format)
 {
-	for (auto &b: bayer_map) {
+	for (auto &b : bayer_map) {
 		if (b.format == format)
 			return b.name;
 	}
@@ -293,7 +293,7 @@
 static PixelFormat
 bayer_format_from_string(const gchar *name)
 {
-	for (auto &b: bayer_map) {
+	for (auto &b : bayer_map) {
 		if (strcmp(b.name, name) == 0)
 			return b.format;
 	}
---
3 potential issues detected, please review
Cleaning up project directory and file based variables 00:02
ERROR: Job failed: exit code 1
```

The no Signed-off-by is odd - as I can see it there, so I'll check that.
Ah - it looks like the patch was missing the '---' separator before the
changelog. I'll fix that.

I'll also add the appropriate spaces when merging...

Johannes, for future patches, please add the post-commit hook to your
libcamera repository to run the checkstyle locally:

--
Kieran


> 
> --
> Kieran
> 
> 
> > 
> > > 
> > > Changes in v3:
> > > - commit msg "parsing" -> "negotiations" and "is"->"it"
> > > - change format argument of bayer_format_to_string to PixelFormat
> > > - make bayer_format_to_string return nullptr if no entry found
> > > - make bayer_formats const
> > > - use range-based for loops
> > > 
> > > Changes in v2:
> > > - Coding style changes
> > > - Removed printf that slipped in
> > > ---
> > >  src/gstreamer/gstlibcamera-utils.cpp | 87 ++++++++++++++--------------
> > >  1 file changed, 44 insertions(+), 43 deletions(-)
> > > 
> > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > > index 79f71246..0820c4b8 100644
> > > --- a/src/gstreamer/gstlibcamera-utils.cpp
> > > +++ b/src/gstreamer/gstlibcamera-utils.cpp
> > > @@ -254,52 +254,50 @@ gst_format_to_pixel_format(GstVideoFormat gst_format)
> > >       return PixelFormat{};
> > >  }
> > >  
> > > +static const struct {
> > > +     PixelFormat format;
> > > +     const gchar *name;
> > > +} bayer_map[]{
> > > +     { formats::SBGGR8, "bggr" },
> > > +     { formats::SGBRG8, "gbrg" },
> > > +     { formats::SGRBG8, "grbg" },
> > > +     { formats::SRGGB8, "rggb" },
> > > +     { formats::SBGGR10, "bggr10le" },
> > > +     { formats::SGBRG10, "gbrg10le" },
> > > +     { formats::SGRBG10, "grbg10le" },
> > > +     { formats::SRGGB10, "rggb10le" },
> > > +     { formats::SBGGR12, "bggr12le" },
> > > +     { formats::SGBRG12, "gbrg12le" },
> > > +     { formats::SGRBG12, "grbg12le" },
> > > +     { formats::SRGGB12, "rggb12le" },
> > > +     { formats::SBGGR14, "bggr14le" },
> > > +     { formats::SGBRG14, "gbrg14le" },
> > > +     { formats::SGRBG14, "grbg14le" },
> > > +     { formats::SRGGB14, "rggb14le" },
> > > +     { formats::SBGGR16, "bggr16le" },
> > > +     { formats::SGBRG16, "gbrg16le" },
> > > +     { formats::SGRBG16, "grbg16le" },
> > > +     { formats::SRGGB16, "rggb16le" },
> > > +};
> > > +
> > >  static const gchar *
> > > -bayer_format_to_string(int format)
> > > +bayer_format_to_string(PixelFormat format)
> > >  {
> > > -     switch (format) {
> > > -     case formats::SBGGR8:
> > > -             return "bggr";
> > > -     case formats::SGBRG8:
> > > -             return "gbrg";
> > > -     case formats::SGRBG8:
> > > -             return "grbg";
> > > -     case formats::SRGGB8:
> > > -             return "rggb";
> > > -     case formats::SBGGR10:
> > > -             return "bggr10le";
> > > -     case formats::SGBRG10:
> > > -             return "gbrg10le";
> > > -     case formats::SGRBG10:
> > > -             return "grbg10le";
> > > -     case formats::SRGGB10:
> > > -             return "rggb10le";
> > > -     case formats::SBGGR12:
> > > -             return "bggr12le";
> > > -     case formats::SGBRG12:
> > > -             return "gbrg12le";
> > > -     case formats::SGRBG12:
> > > -             return "grbg12le";
> > > -     case formats::SRGGB12:
> > > -             return "rggb12le";
> > > -     case formats::SBGGR14:
> > > -             return "bggr14le";
> > > -     case formats::SGBRG14:
> > > -             return "gbrg14le";
> > > -     case formats::SGRBG14:
> > > -             return "grbg14le";
> > > -     case formats::SRGGB14:
> > > -             return "rggb14le";
> > > -     case formats::SBGGR16:
> > > -             return "bggr16le";
> > > -     case formats::SGBRG16:
> > > -             return "gbrg16le";
> > > -     case formats::SGRBG16:
> > > -             return "grbg16le";
> > > -     case formats::SRGGB16:
> > > -             return "rggb16le";
> > > +     for (auto &b: bayer_map) {
> > > +             if (b.format == format)
> > > +                     return b.name;
> > >       }
> > > -     return NULL;
> > > +     return nullptr;
> > > +}
> > > +
> > > +static PixelFormat
> > > +bayer_format_from_string(const gchar *name)
> > > +{
> > > +     for (auto &b: bayer_map) {
> > > +             if (strcmp(b.name, name) == 0)
> > > +                     return b.format;
> > > +     }
> > > +     return PixelFormat{};
> > >  }
> > >  
> > >  static GstStructure *
> > > @@ -474,6 +472,9 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
> > >               const gchar *format = gst_structure_get_string(s, "format");
> > >               gst_format = gst_video_format_from_string(format);
> > >               stream_cfg.pixelFormat = gst_format_to_pixel_format(gst_format);
> > > +     } else if (gst_structure_has_name(s, "video/x-bayer")) {
> > > +             const gchar *format = gst_structure_get_string(s, "format");
> > > +             stream_cfg.pixelFormat = bayer_format_from_string(format);
> > >       } else if (gst_structure_has_name(s, "image/jpeg")) {
> > >               stream_cfg.pixelFormat = formats::MJPEG;
> > >       } else {
> >


More information about the libcamera-devel mailing list