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

[Kieran Bingham]

Hi Johannes,

Quoting mailinglist1 at johanneskirchmair.de (2024-10-31 08:16:45)
> From: Johannes Kirchmair <johannes.kirchmair at skidata.com>
> 
> The parsing of video/x-bayer sources from string makes is 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>

Thanks, this sounds like a good improvement!


> ---
>  src/gstreamer/gstlibcamera-utils.cpp | 90 ++++++++++++++--------------
>  1 file changed, 46 insertions(+), 44 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index 79f71246..472367f2 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -254,54 +254,53 @@ gst_format_to_pixel_format(GstVideoFormat gst_format)
>         return PixelFormat{};
>  }
>  
> +static struct {
> +       PixelFormat format;
> +       const gchar *name;
> +} bayer_formats[]{
> +       { 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" },
> +};

I see we already have format_map[] defining a large part of this table.

But aside from additions to bayer2rgb and rgb2bayer, I don't see
anything defining a specific GST_VIDEO_ type for bayer formats so it
seems like this might be the cleanest way to handle this for now...

Have you tested that this interacts correctly with the bayer2rgb
element?

> +
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
> +
>  static const gchar *
>  bayer_format_to_string(int 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 (unsigned int i = 0; i < ARRAY_SIZE(bayer_formats); i++) {
> +               if ((uint32_t)bayer_formats[i].format == (uint32_t)format)
> +                       return bayer_formats[i].name;
>         }
>         return NULL;
>  }
>  
> +static PixelFormat bayer_format_from_string(const gchar *name)

Looking at the existing coding style throughout this file, i think this
should be:

static PixelFormat
bayer_format_from_string(const gchar *name)

...


> +{
> +       for (unsigned int i = 0; i < ARRAY_SIZE(bayer_formats); i++) {
> +               if (strcmp(bayer_formats[i].name, name) == 0)
> +                       return bayer_formats[i].format;
> +       }
> +       return PixelFormat{};
> +}
> +
>  static GstStructure *
>  bare_structure_from_format(const PixelFormat &format)
>  {
> @@ -407,9 +406,8 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg
>         return caps;
>  }
>  
> -void
> -gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
> -                                        GstCaps *caps)
> +void gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
> +                                             GstCaps *caps)

And this shouldn't be changed.


>  {
>         GstVideoFormat gst_format = pixel_format_to_gst_format(stream_cfg.pixelFormat);
>         guint i;
> @@ -469,11 +467,15 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
>                 gst_structure_fixate_field_string(s, "format", format);
>         }
>  
> +       printf("in the function!!!!!!");

And I think we need to remove this too ...

--
Regards

Kieran


>         /* Then configure the stream with the result. */
>         if (gst_structure_has_name(s, "video/x-raw")) {
>                 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 {
> -- 
> 2.34.1
>