[v2] gstreamer: Add support for Orientation
diff mbox series

Message ID 20250723093954.599102-1-giacomo.cappellini.87@gmail.com
State Superseded
Headers show
Series
  • [v2] gstreamer: Add support for Orientation
Related show

Commit Message

Giacomo Cappellini July 23, 2025, 9:39 a.m. UTC 
libcamera allows to control the images orientation through the
CameraConfiguration::orientation field, expose a GST_PARAM_MUTABLE_READY
parameter of type GstVideoOrientationMethod in GstLibcameraSrc to
control it. Parameter is mapped internally to libcamera::Orientation via
new gstlibcamera-utils functions:
- gst_video_orientation_to_libcamera_orientation
- libcamera_orientation_to_gst_video_orientation

Update CameraConfiguration::Adjusted case in negotiation to warn about
changes in StreamConfiguration and SensorConfiguration, as well as
the new Orientation parameter.

signed-off-by: Giacomo Cappellini <giacomo.cappellini.87@gmail.com>
---
 src/gstreamer/gstlibcamera-utils.cpp |  32 +++++++
 src/gstreamer/gstlibcamera-utils.h   |   4 +
 src/gstreamer/gstlibcamerasrc.cpp    | 124 ++++++++++++++++++++++++++-
 3 files changed, 159 insertions(+), 1 deletion(-)

Comments

Umang Jain July 23, 2025, 11:38 a.m. UTC | #1 
Hello Giacomo

On Wed, Jul 23, 2025 at 11:39:37AM +0200, Giacomo Cappellini wrote:
> libcamera allows to control the images orientation through the
> CameraConfiguration::orientation field, expose a GST_PARAM_MUTABLE_READY
> parameter of type GstVideoOrientationMethod in GstLibcameraSrc to
> control it. Parameter is mapped internally to libcamera::Orientation via
> new gstlibcamera-utils functions:
> - gst_video_orientation_to_libcamera_orientation
> - libcamera_orientation_to_gst_video_orientation
> 
> Update CameraConfiguration::Adjusted case in negotiation to warn about
> changes in StreamConfiguration and SensorConfiguration, as well as
> the new Orientation parameter.
> 
> signed-off-by: Giacomo Cappellini <giacomo.cappellini.87@gmail.com>
> ---
>  src/gstreamer/gstlibcamera-utils.cpp |  32 +++++++
>  src/gstreamer/gstlibcamera-utils.h   |   4 +
>  src/gstreamer/gstlibcamerasrc.cpp    | 124 ++++++++++++++++++++++++++-
>  3 files changed, 159 insertions(+), 1 deletion(-)
> 

Please run checkstyle for your patch before sending. There are many
issues with this patch
($) ./utils/checkstyle.py HEAD

> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index a548b0c1..fc29d19d 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -10,6 +10,8 @@
>  
>  #include <libcamera/control_ids.h>
>  #include <libcamera/formats.h>
> +#include <libcamera/orientation.h>
> +#include <gst/video/video.h>
>  
>  using namespace libcamera;
>  
> @@ -659,3 +661,33 @@ gst_libcamera_get_camera_manager(int &ret)
>  
>  	return cm;
>  }
> +
> +Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)
> +{
> +    switch (method) {
> +    case GST_VIDEO_ORIENTATION_IDENTITY:   return Orientation::Rotate0;
> +    case GST_VIDEO_ORIENTATION_90R:        return Orientation::Rotate90;
> +    case GST_VIDEO_ORIENTATION_180:        return Orientation::Rotate180;
> +    case GST_VIDEO_ORIENTATION_90L:        return Orientation::Rotate270;
> +    case GST_VIDEO_ORIENTATION_HORIZ:      return Orientation::Rotate0Mirror;
> +    case GST_VIDEO_ORIENTATION_VERT:       return Orientation::Rotate180Mirror;
> +    case GST_VIDEO_ORIENTATION_UL_LR:      return Orientation::Rotate90Mirror;
> +    case GST_VIDEO_ORIENTATION_UR_LL:      return Orientation::Rotate270Mirror;
> +    default:                               return Orientation::Rotate0;
> +    }
> +}
> +
> +GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(Orientation orientation)
> +{
> +    switch (orientation) {
> +    case Orientation::Rotate0:           return GST_VIDEO_ORIENTATION_IDENTITY;
> +    case Orientation::Rotate90:          return GST_VIDEO_ORIENTATION_90R;
> +    case Orientation::Rotate180:         return GST_VIDEO_ORIENTATION_180;
> +    case Orientation::Rotate270:         return GST_VIDEO_ORIENTATION_90L;
> +    case Orientation::Rotate0Mirror:     return GST_VIDEO_ORIENTATION_HORIZ;
> +    case Orientation::Rotate180Mirror:   return GST_VIDEO_ORIENTATION_VERT;
> +    case Orientation::Rotate90Mirror:    return GST_VIDEO_ORIENTATION_UL_LR;
> +    case Orientation::Rotate270Mirror:   return GST_VIDEO_ORIENTATION_UR_LL;
> +    default:                             return GST_VIDEO_ORIENTATION_IDENTITY;
> +    }
> +}

We could something similar to buffer_map[] and format_map[] in the same
file. This way the mappings are stated only once and would be easier
to update later just in case.

static const struct {                                                           
        Orientation orientation;                                                
        GstVideoOrientationMethod method;                                       
} orientation_map[]{                                                            
        { Orientation::Rotate0, GST_VIDEO_ORIENTATION_IDENTITY },               
        { Orientation::Rotate90, GST_VIDEO_ORIENTATION_90R },                   
        { Orientation::Rotate180, GST_VIDEO_ORIENTATION_180 },                  
        { Orientation::Rotate270, GST_VIDEO_ORIENTATION_90L },                  
        { Orientation::Rotate0Mirror, GST_VIDEO_ORIENTATION_HORIZ },            
        { Orientation::Rotate180Mirror, GST_VIDEO_ORIENTATION_VERT },           
        { Orientation::Rotate90Mirror, GST_VIDEO_ORIENTATION_UL_LR },           
        { Orientation::Rotate270Mirror, GST_VIDEO_ORIENTATION_UR_LL },          
};                                                                              
                                                                                
Orientation                                                                     
gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)
{                                                                               
        for (auto &b : orientation_map) {                                       
                if (b.method == method)                                         
                        return b.orientation;                                   
        }                                                                       
                                                                                
        return Orientation::Rotate0;                                            
                                                                                
}                                                                               
                                                                                
GstVideoOrientationMethod                                                       
libcamera_orientation_to_gst_video_orientation(Orientation orientation)         
{                                                                               
        for (auto &a : orientation_map) {                                       
                if (a.orientation == orientation)                               
                        return a.method;                                        
        }                                                                       
                                                                                
        return GST_VIDEO_ORIENTATION_IDENTITY;                                  
}  

> \ No newline at end of file
> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h
> index 5f4e8a0f..3d4b049f 100644
> --- a/src/gstreamer/gstlibcamera-utils.h
> +++ b/src/gstreamer/gstlibcamera-utils.h
> @@ -11,6 +11,7 @@
>  #include <libcamera/camera_manager.h>
>  #include <libcamera/controls.h>
>  #include <libcamera/stream.h>
> +#include <libcamera/orientation.h>
>  
>  #include <gst/gst.h>
>  #include <gst/video/video.h>
> @@ -92,3 +93,6 @@ public:
>  private:
>  	GRecMutex *mutex_;
>  };
> +
> +libcamera::Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method);
> +GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(libcamera::Orientation orientation);
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 3aca4eed..03d389aa 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -32,12 +32,14 @@
>  #include <tuple>
>  #include <utility>
>  #include <vector>
> +#include <sstream>
>  
>  #include <libcamera/camera.h>
>  #include <libcamera/camera_manager.h>
>  #include <libcamera/control_ids.h>
>  
>  #include <gst/base/base.h>
> +#include <gst/video/video.h>
>  
>  #include "gstlibcamera-controls.h"
>  #include "gstlibcamera-utils.h"
> @@ -146,6 +148,7 @@ struct _GstLibcameraSrc {
>  	GstTask *task;
>  
>  	gchar *camera_name;
> +	GstVideoOrientationMethod orientation;
>  
>  	std::atomic<GstEvent *> pending_eos;
>  
> @@ -157,6 +160,7 @@ struct _GstLibcameraSrc {
>  enum {
>  	PROP_0,
>  	PROP_CAMERA_NAME,
> +	PROP_ORIENTATION,
>  	PROP_LAST
>  };
>  
> @@ -616,9 +620,110 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>  		gst_libcamera_get_framerate_from_caps(caps, element_caps);
>  	}
>  
> +	/* Set orientation control. */
> +	state->config_->orientation = gst_video_orientation_to_libcamera_orientation(self->orientation);
> +
> +	/* Save original configuration for comparison after validation */
> +	std::vector<StreamConfiguration> orig_stream_cfgs;
> +	for (gsize i = 0; i < state->config_->size(); i++)
> +		orig_stream_cfgs.push_back(state->config_->at(i));
> +	std::optional<SensorConfiguration> orig_sensor_cfg = state->config_->sensorConfig;
> +	Orientation orig_orientation = state->config_->orientation;
> +
>  	/* Validate the configuration. */
> -	if (state->config_->validate() == CameraConfiguration::Invalid)
> +	switch(state->config_->validate()) {
> +	case CameraConfiguration::Valid:
> +		GST_DEBUG_OBJECT(self, "Camera configuration is valid");
> +		break;
> +	case CameraConfiguration::Adjusted:

Did you happen to see: https://patchwork.libcamera.org/patch/23898/ ?

> +	{	
> +		bool warned = false;
> +		// Warn if number of StreamConfigurations changed
> +		if (orig_stream_cfgs.size() != state->config_->size()) {
> +			GST_WARNING_OBJECT(self, "Number of StreamConfiguration elements changed: requested=%zu, actual=%zu",
> +				orig_stream_cfgs.size(), state->config_->size());
> +			warned = true;
> +		}
> +		// Warn about changes in each StreamConfiguration
> +		// TODO implement diffing in StreamConfiguration 
> +		for (gsize i = 0; i < std::min(orig_stream_cfgs.size(), state->config_->size()); i++) {
> +			if (orig_stream_cfgs[i].toString() != state->config_->at(i).toString()) {
> +				GST_WARNING_OBJECT(self, "StreamConfiguration %zu changed: %s -> %s",
> +					i, orig_stream_cfgs[i].toString().c_str(),
> +					state->config_->at(i).toString().c_str());
> +				warned = true;
> +			}
> +		}
> +		// Warn about SensorConfiguration changes
> +		// TODO implement diffing in SensorConfiguration
> +		if (orig_sensor_cfg.has_value() || state->config_->sensorConfig.has_value()) {
> +			const SensorConfiguration *orig = orig_sensor_cfg.has_value() ? &orig_sensor_cfg.value() : nullptr;
> +			const SensorConfiguration *curr = state->config_->sensorConfig.has_value() ? &state->config_->sensorConfig.value() : nullptr;
> +			bool sensor_changed = false;
> +			std::ostringstream diff;
> +			if ((orig == nullptr) != (curr == nullptr)) {
> +				diff << "SensorConfiguration presence changed: "
> +				     << (orig ? "was present" : "was absent")
> +				     << " -> "
> +				     << (curr ? "present" : "absent");
> +				sensor_changed = true;
> +			} else if (orig && curr) {
> +				if (orig->bitDepth != curr->bitDepth) {
> +					diff << "bitDepth: " << orig->bitDepth << " -> " << curr->bitDepth << "; ";
> +					sensor_changed = true;
> +				}
> +				if (orig->analogCrop != curr->analogCrop) {
> +					diff << "analogCrop: " << orig->analogCrop.toString() << " -> " << curr->analogCrop.toString() << "; ";
> +					sensor_changed = true;
> +				}
> +				if (orig->binning.binX != curr->binning.binX ||
> +				    orig->binning.binY != curr->binning.binY) {
> +					diff << "binning: (" << orig->binning.binX << "," << orig->binning.binY << ") -> ("
> +					     << curr->binning.binX << "," << curr->binning.binY << "); ";
> +					sensor_changed = true;
> +				}
> +				if (orig->skipping.xOddInc != curr->skipping.xOddInc ||
> +				    orig->skipping.xEvenInc != curr->skipping.xEvenInc ||
> +				    orig->skipping.yOddInc != curr->skipping.yOddInc ||
> +				    orig->skipping.yEvenInc != curr->skipping.yEvenInc) {
> +					diff << "skipping: ("
> +					     << orig->skipping.xOddInc << "," << orig->skipping.xEvenInc << ","
> +					     << orig->skipping.yOddInc << "," << orig->skipping.yEvenInc << ") -> ("
> +					     << curr->skipping.xOddInc << "," << curr->skipping.xEvenInc << ","
> +					     << curr->skipping.yOddInc << "," << curr->skipping.yEvenInc << "); ";
> +					sensor_changed = true;
> +				}
> +				if (orig->outputSize != curr->outputSize) {
> +					diff << "outputSize: " << orig->outputSize.toString() << " -> " << curr->outputSize.toString() << "; ";
> +					sensor_changed = true;
> +				}
> +			}
> +			if (sensor_changed) {
> +				GST_WARNING_OBJECT(self, "SensorConfiguration changed: %s", diff.str().c_str());
> +				warned = true;
> +			}
> +		}
> +		// Warn about orientation change
> +		if (orig_orientation != state->config_->orientation) {
> +			GEnumClass *enum_class = (GEnumClass *)g_type_class_ref(GST_TYPE_VIDEO_ORIENTATION_METHOD);
> +			const char *orig_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(orig_orientation))->value_nick;
> +			const char *new_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(state->config_->orientation))->value_nick;
> +			GST_WARNING_OBJECT(self, "Orientation changed: %s -> %s", orig_orientation_str, new_orientation_str);
> +			warned = true;
> +		}
> +		if (!warned) {
> +			GST_DEBUG_OBJECT(self, "Camera configuration adjusted, but no significant changes detected.");
> +		}
> +		// Update Gst orientation property to match adjusted config
> +		self->orientation = libcamera_orientation_to_gst_video_orientation(state->config_->orientation);
> +		break;
> +	}
> +	case CameraConfiguration::Invalid:
> +		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> +				  ("Camera configuration is not supported"),
> +				  ("CameraConfiguration::validate() returned Invalid"));
>  		return false;
> +	}
>  
>  	int ret = state->cam_->configure(state->config_.get());
>  	if (ret) {
> @@ -926,6 +1031,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
>  		g_free(self->camera_name);
>  		self->camera_name = g_value_dup_string(value);
>  		break;
> +	case PROP_ORIENTATION:
> +		self->orientation = (GstVideoOrientationMethod)g_value_get_enum(value);
> +		break;
>  	default:
>  		if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
>  			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> @@ -945,6 +1053,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
>  	case PROP_CAMERA_NAME:
>  		g_value_set_string(value, self->camera_name);
>  		break;
> +	case PROP_ORIENTATION:
> +		g_value_set_enum(value, (gint)self->orientation);
> +		break;
>  	default:
>  		if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
>  			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> @@ -1154,6 +1265,17 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>  							     | G_PARAM_STATIC_STRINGS));
>  	g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
>  
> +	/* Register the orientation enum type. */
> +	spec = g_param_spec_enum("orientation", "Orientation",
> +					       "Select the orientation of the camera.",
> +					       GST_TYPE_VIDEO_ORIENTATION_METHOD,
> +					       GST_VIDEO_ORIENTATION_IDENTITY,
> +					       (GParamFlags)(GST_PARAM_MUTABLE_READY
> +							     | G_PARAM_CONSTRUCT
> +							     | G_PARAM_READWRITE
> +							     | G_PARAM_STATIC_STRINGS));
> +	g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
> +
>  	GstCameraControls::installProperties(object_class, PROP_LAST);
>  }
>  
> -- 
> 2.43.0
>
Giacomo Cappellini July 23, 2025, 2:07 p.m. UTC | #2 
Thanks for the review.

I was not aware of https://patchwork.libcamera.org/patch/23898/, we
made changes to gst_libcamera_src_negotiate at same time/day.
I see my contribution brings a more compete diffing for the
CameraConfiguration::Adjusted case, but my code lacks the check if
downstream can accept this new stream configuration and if
not return a not-negotiated error.

In v3 I've fixed the style errors and replaced old maps with
orientation_map as you suggested.

G.C.

Il giorno mer 23 lug 2025 alle ore 13:37 Umang Jain
<uajain@igalia.com> ha scritto:
>
> Hello Giacomo
>
> On Wed, Jul 23, 2025 at 11:39:37AM +0200, Giacomo Cappellini wrote:
> > libcamera allows to control the images orientation through the
> > CameraConfiguration::orientation field, expose a GST_PARAM_MUTABLE_READY
> > parameter of type GstVideoOrientationMethod in GstLibcameraSrc to
> > control it. Parameter is mapped internally to libcamera::Orientation via
> > new gstlibcamera-utils functions:
> > - gst_video_orientation_to_libcamera_orientation
> > - libcamera_orientation_to_gst_video_orientation
> >
> > Update CameraConfiguration::Adjusted case in negotiation to warn about
> > changes in StreamConfiguration and SensorConfiguration, as well as
> > the new Orientation parameter.
> >
> > signed-off-by: Giacomo Cappellini <giacomo.cappellini.87@gmail.com>
> > ---
> >  src/gstreamer/gstlibcamera-utils.cpp |  32 +++++++
> >  src/gstreamer/gstlibcamera-utils.h   |   4 +
> >  src/gstreamer/gstlibcamerasrc.cpp    | 124 ++++++++++++++++++++++++++-
> >  3 files changed, 159 insertions(+), 1 deletion(-)
> >
>
> Please run checkstyle for your patch before sending. There are many
> issues with this patch
> ($) ./utils/checkstyle.py HEAD
>
> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > index a548b0c1..fc29d19d 100644
> > --- a/src/gstreamer/gstlibcamera-utils.cpp
> > +++ b/src/gstreamer/gstlibcamera-utils.cpp
> > @@ -10,6 +10,8 @@
> >
> >  #include <libcamera/control_ids.h>
> >  #include <libcamera/formats.h>
> > +#include <libcamera/orientation.h>
> > +#include <gst/video/video.h>
> >
> >  using namespace libcamera;
> >
> > @@ -659,3 +661,33 @@ gst_libcamera_get_camera_manager(int &ret)
> >
> >       return cm;
> >  }
> > +
> > +Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)
> > +{
> > +    switch (method) {
> > +    case GST_VIDEO_ORIENTATION_IDENTITY:   return Orientation::Rotate0;
> > +    case GST_VIDEO_ORIENTATION_90R:        return Orientation::Rotate90;
> > +    case GST_VIDEO_ORIENTATION_180:        return Orientation::Rotate180;
> > +    case GST_VIDEO_ORIENTATION_90L:        return Orientation::Rotate270;
> > +    case GST_VIDEO_ORIENTATION_HORIZ:      return Orientation::Rotate0Mirror;
> > +    case GST_VIDEO_ORIENTATION_VERT:       return Orientation::Rotate180Mirror;
> > +    case GST_VIDEO_ORIENTATION_UL_LR:      return Orientation::Rotate90Mirror;
> > +    case GST_VIDEO_ORIENTATION_UR_LL:      return Orientation::Rotate270Mirror;
> > +    default:                               return Orientation::Rotate0;
> > +    }
> > +}
> > +
> > +GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(Orientation orientation)
> > +{
> > +    switch (orientation) {
> > +    case Orientation::Rotate0:           return GST_VIDEO_ORIENTATION_IDENTITY;
> > +    case Orientation::Rotate90:          return GST_VIDEO_ORIENTATION_90R;
> > +    case Orientation::Rotate180:         return GST_VIDEO_ORIENTATION_180;
> > +    case Orientation::Rotate270:         return GST_VIDEO_ORIENTATION_90L;
> > +    case Orientation::Rotate0Mirror:     return GST_VIDEO_ORIENTATION_HORIZ;
> > +    case Orientation::Rotate180Mirror:   return GST_VIDEO_ORIENTATION_VERT;
> > +    case Orientation::Rotate90Mirror:    return GST_VIDEO_ORIENTATION_UL_LR;
> > +    case Orientation::Rotate270Mirror:   return GST_VIDEO_ORIENTATION_UR_LL;
> > +    default:                             return GST_VIDEO_ORIENTATION_IDENTITY;
> > +    }
> > +}
>
> We could something similar to buffer_map[] and format_map[] in the same
> file. This way the mappings are stated only once and would be easier
> to update later just in case.
>
> static const struct {
>         Orientation orientation;
>         GstVideoOrientationMethod method;
> } orientation_map[]{
>         { Orientation::Rotate0, GST_VIDEO_ORIENTATION_IDENTITY },
>         { Orientation::Rotate90, GST_VIDEO_ORIENTATION_90R },
>         { Orientation::Rotate180, GST_VIDEO_ORIENTATION_180 },
>         { Orientation::Rotate270, GST_VIDEO_ORIENTATION_90L },
>         { Orientation::Rotate0Mirror, GST_VIDEO_ORIENTATION_HORIZ },
>         { Orientation::Rotate180Mirror, GST_VIDEO_ORIENTATION_VERT },
>         { Orientation::Rotate90Mirror, GST_VIDEO_ORIENTATION_UL_LR },
>         { Orientation::Rotate270Mirror, GST_VIDEO_ORIENTATION_UR_LL },
> };
>
> Orientation
> gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)
> {
>         for (auto &b : orientation_map) {
>                 if (b.method == method)
>                         return b.orientation;
>         }
>
>         return Orientation::Rotate0;
>
> }
>
> GstVideoOrientationMethod
> libcamera_orientation_to_gst_video_orientation(Orientation orientation)
> {
>         for (auto &a : orientation_map) {
>                 if (a.orientation == orientation)
>                         return a.method;
>         }
>
>         return GST_VIDEO_ORIENTATION_IDENTITY;
> }
>
> > \ No newline at end of file
> > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h
> > index 5f4e8a0f..3d4b049f 100644
> > --- a/src/gstreamer/gstlibcamera-utils.h
> > +++ b/src/gstreamer/gstlibcamera-utils.h
> > @@ -11,6 +11,7 @@
> >  #include <libcamera/camera_manager.h>
> >  #include <libcamera/controls.h>
> >  #include <libcamera/stream.h>
> > +#include <libcamera/orientation.h>
> >
> >  #include <gst/gst.h>
> >  #include <gst/video/video.h>
> > @@ -92,3 +93,6 @@ public:
> >  private:
> >       GRecMutex *mutex_;
> >  };
> > +
> > +libcamera::Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method);
> > +GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(libcamera::Orientation orientation);
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 3aca4eed..03d389aa 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -32,12 +32,14 @@
> >  #include <tuple>
> >  #include <utility>
> >  #include <vector>
> > +#include <sstream>
> >
> >  #include <libcamera/camera.h>
> >  #include <libcamera/camera_manager.h>
> >  #include <libcamera/control_ids.h>
> >
> >  #include <gst/base/base.h>
> > +#include <gst/video/video.h>
> >
> >  #include "gstlibcamera-controls.h"
> >  #include "gstlibcamera-utils.h"
> > @@ -146,6 +148,7 @@ struct _GstLibcameraSrc {
> >       GstTask *task;
> >
> >       gchar *camera_name;
> > +     GstVideoOrientationMethod orientation;
> >
> >       std::atomic<GstEvent *> pending_eos;
> >
> > @@ -157,6 +160,7 @@ struct _GstLibcameraSrc {
> >  enum {
> >       PROP_0,
> >       PROP_CAMERA_NAME,
> > +     PROP_ORIENTATION,
> >       PROP_LAST
> >  };
> >
> > @@ -616,9 +620,110 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
> >               gst_libcamera_get_framerate_from_caps(caps, element_caps);
> >       }
> >
> > +     /* Set orientation control. */
> > +     state->config_->orientation = gst_video_orientation_to_libcamera_orientation(self->orientation);
> > +
> > +     /* Save original configuration for comparison after validation */
> > +     std::vector<StreamConfiguration> orig_stream_cfgs;
> > +     for (gsize i = 0; i < state->config_->size(); i++)
> > +             orig_stream_cfgs.push_back(state->config_->at(i));
> > +     std::optional<SensorConfiguration> orig_sensor_cfg = state->config_->sensorConfig;
> > +     Orientation orig_orientation = state->config_->orientation;
> > +
> >       /* Validate the configuration. */
> > -     if (state->config_->validate() == CameraConfiguration::Invalid)
> > +     switch(state->config_->validate()) {
> > +     case CameraConfiguration::Valid:
> > +             GST_DEBUG_OBJECT(self, "Camera configuration is valid");
> > +             break;
> > +     case CameraConfiguration::Adjusted:
>
> Did you happen to see: https://patchwork.libcamera.org/patch/23898/ ?
>
> > +     {
> > +             bool warned = false;
> > +             // Warn if number of StreamConfigurations changed
> > +             if (orig_stream_cfgs.size() != state->config_->size()) {
> > +                     GST_WARNING_OBJECT(self, "Number of StreamConfiguration elements changed: requested=%zu, actual=%zu",
> > +                             orig_stream_cfgs.size(), state->config_->size());
> > +                     warned = true;
> > +             }
> > +             // Warn about changes in each StreamConfiguration
> > +             // TODO implement diffing in StreamConfiguration
> > +             for (gsize i = 0; i < std::min(orig_stream_cfgs.size(), state->config_->size()); i++) {
> > +                     if (orig_stream_cfgs[i].toString() != state->config_->at(i).toString()) {
> > +                             GST_WARNING_OBJECT(self, "StreamConfiguration %zu changed: %s -> %s",
> > +                                     i, orig_stream_cfgs[i].toString().c_str(),
> > +                                     state->config_->at(i).toString().c_str());
> > +                             warned = true;
> > +                     }
> > +             }
> > +             // Warn about SensorConfiguration changes
> > +             // TODO implement diffing in SensorConfiguration
> > +             if (orig_sensor_cfg.has_value() || state->config_->sensorConfig.has_value()) {
> > +                     const SensorConfiguration *orig = orig_sensor_cfg.has_value() ? &orig_sensor_cfg.value() : nullptr;
> > +                     const SensorConfiguration *curr = state->config_->sensorConfig.has_value() ? &state->config_->sensorConfig.value() : nullptr;
> > +                     bool sensor_changed = false;
> > +                     std::ostringstream diff;
> > +                     if ((orig == nullptr) != (curr == nullptr)) {
> > +                             diff << "SensorConfiguration presence changed: "
> > +                                  << (orig ? "was present" : "was absent")
> > +                                  << " -> "
> > +                                  << (curr ? "present" : "absent");
> > +                             sensor_changed = true;
> > +                     } else if (orig && curr) {
> > +                             if (orig->bitDepth != curr->bitDepth) {
> > +                                     diff << "bitDepth: " << orig->bitDepth << " -> " << curr->bitDepth << "; ";
> > +                                     sensor_changed = true;
> > +                             }
> > +                             if (orig->analogCrop != curr->analogCrop) {
> > +                                     diff << "analogCrop: " << orig->analogCrop.toString() << " -> " << curr->analogCrop.toString() << "; ";
> > +                                     sensor_changed = true;
> > +                             }
> > +                             if (orig->binning.binX != curr->binning.binX ||
> > +                                 orig->binning.binY != curr->binning.binY) {
> > +                                     diff << "binning: (" << orig->binning.binX << "," << orig->binning.binY << ") -> ("
> > +                                          << curr->binning.binX << "," << curr->binning.binY << "); ";
> > +                                     sensor_changed = true;
> > +                             }
> > +                             if (orig->skipping.xOddInc != curr->skipping.xOddInc ||
> > +                                 orig->skipping.xEvenInc != curr->skipping.xEvenInc ||
> > +                                 orig->skipping.yOddInc != curr->skipping.yOddInc ||
> > +                                 orig->skipping.yEvenInc != curr->skipping.yEvenInc) {
> > +                                     diff << "skipping: ("
> > +                                          << orig->skipping.xOddInc << "," << orig->skipping.xEvenInc << ","
> > +                                          << orig->skipping.yOddInc << "," << orig->skipping.yEvenInc << ") -> ("
> > +                                          << curr->skipping.xOddInc << "," << curr->skipping.xEvenInc << ","
> > +                                          << curr->skipping.yOddInc << "," << curr->skipping.yEvenInc << "); ";
> > +                                     sensor_changed = true;
> > +                             }
> > +                             if (orig->outputSize != curr->outputSize) {
> > +                                     diff << "outputSize: " << orig->outputSize.toString() << " -> " << curr->outputSize.toString() << "; ";
> > +                                     sensor_changed = true;
> > +                             }
> > +                     }
> > +                     if (sensor_changed) {
> > +                             GST_WARNING_OBJECT(self, "SensorConfiguration changed: %s", diff.str().c_str());
> > +                             warned = true;
> > +                     }
> > +             }
> > +             // Warn about orientation change
> > +             if (orig_orientation != state->config_->orientation) {
> > +                     GEnumClass *enum_class = (GEnumClass *)g_type_class_ref(GST_TYPE_VIDEO_ORIENTATION_METHOD);
> > +                     const char *orig_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(orig_orientation))->value_nick;
> > +                     const char *new_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(state->config_->orientation))->value_nick;
> > +                     GST_WARNING_OBJECT(self, "Orientation changed: %s -> %s", orig_orientation_str, new_orientation_str);
> > +                     warned = true;
> > +             }
> > +             if (!warned) {
> > +                     GST_DEBUG_OBJECT(self, "Camera configuration adjusted, but no significant changes detected.");
> > +             }
> > +             // Update Gst orientation property to match adjusted config
> > +             self->orientation = libcamera_orientation_to_gst_video_orientation(state->config_->orientation);
> > +             break;
> > +     }
> > +     case CameraConfiguration::Invalid:
> > +             GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> > +                               ("Camera configuration is not supported"),
> > +                               ("CameraConfiguration::validate() returned Invalid"));
> >               return false;
> > +     }
> >
> >       int ret = state->cam_->configure(state->config_.get());
> >       if (ret) {
> > @@ -926,6 +1031,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
> >               g_free(self->camera_name);
> >               self->camera_name = g_value_dup_string(value);
> >               break;
> > +     case PROP_ORIENTATION:
> > +             self->orientation = (GstVideoOrientationMethod)g_value_get_enum(value);
> > +             break;
> >       default:
> >               if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
> >                       G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -945,6 +1053,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
> >       case PROP_CAMERA_NAME:
> >               g_value_set_string(value, self->camera_name);
> >               break;
> > +     case PROP_ORIENTATION:
> > +             g_value_set_enum(value, (gint)self->orientation);
> > +             break;
> >       default:
> >               if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
> >                       G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -1154,6 +1265,17 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >                                                            | G_PARAM_STATIC_STRINGS));
> >       g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
> >
> > +     /* Register the orientation enum type. */
> > +     spec = g_param_spec_enum("orientation", "Orientation",
> > +                                            "Select the orientation of the camera.",
> > +                                            GST_TYPE_VIDEO_ORIENTATION_METHOD,
> > +                                            GST_VIDEO_ORIENTATION_IDENTITY,
> > +                                            (GParamFlags)(GST_PARAM_MUTABLE_READY
> > +                                                          | G_PARAM_CONSTRUCT
> > +                                                          | G_PARAM_READWRITE
> > +                                                          | G_PARAM_STATIC_STRINGS));
> > +     g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
> > +
> >       GstCameraControls::installProperties(object_class, PROP_LAST);
> >  }
> >
> > --
> > 2.43.0
> >

Patch
diff mbox series 

diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
index a548b0c1..fc29d19d 100644
--- a/src/gstreamer/gstlibcamera-utils.cpp
+++ b/src/gstreamer/gstlibcamera-utils.cpp
@@ -10,6 +10,8 @@ 
 
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
+#include <libcamera/orientation.h>
+#include <gst/video/video.h>
 
 using namespace libcamera;
 
@@ -659,3 +661,33 @@  gst_libcamera_get_camera_manager(int &ret)
 
 	return cm;
 }
+
+Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method)
+{
+    switch (method) {
+    case GST_VIDEO_ORIENTATION_IDENTITY:   return Orientation::Rotate0;
+    case GST_VIDEO_ORIENTATION_90R:        return Orientation::Rotate90;
+    case GST_VIDEO_ORIENTATION_180:        return Orientation::Rotate180;
+    case GST_VIDEO_ORIENTATION_90L:        return Orientation::Rotate270;
+    case GST_VIDEO_ORIENTATION_HORIZ:      return Orientation::Rotate0Mirror;
+    case GST_VIDEO_ORIENTATION_VERT:       return Orientation::Rotate180Mirror;
+    case GST_VIDEO_ORIENTATION_UL_LR:      return Orientation::Rotate90Mirror;
+    case GST_VIDEO_ORIENTATION_UR_LL:      return Orientation::Rotate270Mirror;
+    default:                               return Orientation::Rotate0;
+    }
+}
+
+GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(Orientation orientation)
+{
+    switch (orientation) {
+    case Orientation::Rotate0:           return GST_VIDEO_ORIENTATION_IDENTITY;
+    case Orientation::Rotate90:          return GST_VIDEO_ORIENTATION_90R;
+    case Orientation::Rotate180:         return GST_VIDEO_ORIENTATION_180;
+    case Orientation::Rotate270:         return GST_VIDEO_ORIENTATION_90L;
+    case Orientation::Rotate0Mirror:     return GST_VIDEO_ORIENTATION_HORIZ;
+    case Orientation::Rotate180Mirror:   return GST_VIDEO_ORIENTATION_VERT;
+    case Orientation::Rotate90Mirror:    return GST_VIDEO_ORIENTATION_UL_LR;
+    case Orientation::Rotate270Mirror:   return GST_VIDEO_ORIENTATION_UR_LL;
+    default:                             return GST_VIDEO_ORIENTATION_IDENTITY;
+    }
+}
\ No newline at end of file
diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h
index 5f4e8a0f..3d4b049f 100644
--- a/src/gstreamer/gstlibcamera-utils.h
+++ b/src/gstreamer/gstlibcamera-utils.h
@@ -11,6 +11,7 @@ 
 #include <libcamera/camera_manager.h>
 #include <libcamera/controls.h>
 #include <libcamera/stream.h>
+#include <libcamera/orientation.h>
 
 #include <gst/gst.h>
 #include <gst/video/video.h>
@@ -92,3 +93,6 @@  public:
 private:
 	GRecMutex *mutex_;
 };
+
+libcamera::Orientation gst_video_orientation_to_libcamera_orientation(GstVideoOrientationMethod method);
+GstVideoOrientationMethod libcamera_orientation_to_gst_video_orientation(libcamera::Orientation orientation);
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 3aca4eed..03d389aa 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -32,12 +32,14 @@ 
 #include <tuple>
 #include <utility>
 #include <vector>
+#include <sstream>
 
 #include <libcamera/camera.h>
 #include <libcamera/camera_manager.h>
 #include <libcamera/control_ids.h>
 
 #include <gst/base/base.h>
+#include <gst/video/video.h>
 
 #include "gstlibcamera-controls.h"
 #include "gstlibcamera-utils.h"
@@ -146,6 +148,7 @@  struct _GstLibcameraSrc {
 	GstTask *task;
 
 	gchar *camera_name;
+	GstVideoOrientationMethod orientation;
 
 	std::atomic<GstEvent *> pending_eos;
 
@@ -157,6 +160,7 @@  struct _GstLibcameraSrc {
 enum {
 	PROP_0,
 	PROP_CAMERA_NAME,
+	PROP_ORIENTATION,
 	PROP_LAST
 };
 
@@ -616,9 +620,110 @@  gst_libcamera_src_negotiate(GstLibcameraSrc *self)
 		gst_libcamera_get_framerate_from_caps(caps, element_caps);
 	}
 
+	/* Set orientation control. */
+	state->config_->orientation = gst_video_orientation_to_libcamera_orientation(self->orientation);
+
+	/* Save original configuration for comparison after validation */
+	std::vector<StreamConfiguration> orig_stream_cfgs;
+	for (gsize i = 0; i < state->config_->size(); i++)
+		orig_stream_cfgs.push_back(state->config_->at(i));
+	std::optional<SensorConfiguration> orig_sensor_cfg = state->config_->sensorConfig;
+	Orientation orig_orientation = state->config_->orientation;
+
 	/* Validate the configuration. */
-	if (state->config_->validate() == CameraConfiguration::Invalid)
+	switch(state->config_->validate()) {
+	case CameraConfiguration::Valid:
+		GST_DEBUG_OBJECT(self, "Camera configuration is valid");
+		break;
+	case CameraConfiguration::Adjusted:
+	{	
+		bool warned = false;
+		// Warn if number of StreamConfigurations changed
+		if (orig_stream_cfgs.size() != state->config_->size()) {
+			GST_WARNING_OBJECT(self, "Number of StreamConfiguration elements changed: requested=%zu, actual=%zu",
+				orig_stream_cfgs.size(), state->config_->size());
+			warned = true;
+		}
+		// Warn about changes in each StreamConfiguration
+		// TODO implement diffing in StreamConfiguration 
+		for (gsize i = 0; i < std::min(orig_stream_cfgs.size(), state->config_->size()); i++) {
+			if (orig_stream_cfgs[i].toString() != state->config_->at(i).toString()) {
+				GST_WARNING_OBJECT(self, "StreamConfiguration %zu changed: %s -> %s",
+					i, orig_stream_cfgs[i].toString().c_str(),
+					state->config_->at(i).toString().c_str());
+				warned = true;
+			}
+		}
+		// Warn about SensorConfiguration changes
+		// TODO implement diffing in SensorConfiguration
+		if (orig_sensor_cfg.has_value() || state->config_->sensorConfig.has_value()) {
+			const SensorConfiguration *orig = orig_sensor_cfg.has_value() ? &orig_sensor_cfg.value() : nullptr;
+			const SensorConfiguration *curr = state->config_->sensorConfig.has_value() ? &state->config_->sensorConfig.value() : nullptr;
+			bool sensor_changed = false;
+			std::ostringstream diff;
+			if ((orig == nullptr) != (curr == nullptr)) {
+				diff << "SensorConfiguration presence changed: "
+				     << (orig ? "was present" : "was absent")
+				     << " -> "
+				     << (curr ? "present" : "absent");
+				sensor_changed = true;
+			} else if (orig && curr) {
+				if (orig->bitDepth != curr->bitDepth) {
+					diff << "bitDepth: " << orig->bitDepth << " -> " << curr->bitDepth << "; ";
+					sensor_changed = true;
+				}
+				if (orig->analogCrop != curr->analogCrop) {
+					diff << "analogCrop: " << orig->analogCrop.toString() << " -> " << curr->analogCrop.toString() << "; ";
+					sensor_changed = true;
+				}
+				if (orig->binning.binX != curr->binning.binX ||
+				    orig->binning.binY != curr->binning.binY) {
+					diff << "binning: (" << orig->binning.binX << "," << orig->binning.binY << ") -> ("
+					     << curr->binning.binX << "," << curr->binning.binY << "); ";
+					sensor_changed = true;
+				}
+				if (orig->skipping.xOddInc != curr->skipping.xOddInc ||
+				    orig->skipping.xEvenInc != curr->skipping.xEvenInc ||
+				    orig->skipping.yOddInc != curr->skipping.yOddInc ||
+				    orig->skipping.yEvenInc != curr->skipping.yEvenInc) {
+					diff << "skipping: ("
+					     << orig->skipping.xOddInc << "," << orig->skipping.xEvenInc << ","
+					     << orig->skipping.yOddInc << "," << orig->skipping.yEvenInc << ") -> ("
+					     << curr->skipping.xOddInc << "," << curr->skipping.xEvenInc << ","
+					     << curr->skipping.yOddInc << "," << curr->skipping.yEvenInc << "); ";
+					sensor_changed = true;
+				}
+				if (orig->outputSize != curr->outputSize) {
+					diff << "outputSize: " << orig->outputSize.toString() << " -> " << curr->outputSize.toString() << "; ";
+					sensor_changed = true;
+				}
+			}
+			if (sensor_changed) {
+				GST_WARNING_OBJECT(self, "SensorConfiguration changed: %s", diff.str().c_str());
+				warned = true;
+			}
+		}
+		// Warn about orientation change
+		if (orig_orientation != state->config_->orientation) {
+			GEnumClass *enum_class = (GEnumClass *)g_type_class_ref(GST_TYPE_VIDEO_ORIENTATION_METHOD);
+			const char *orig_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(orig_orientation))->value_nick;
+			const char *new_orientation_str = g_enum_get_value(enum_class, libcamera_orientation_to_gst_video_orientation(state->config_->orientation))->value_nick;
+			GST_WARNING_OBJECT(self, "Orientation changed: %s -> %s", orig_orientation_str, new_orientation_str);
+			warned = true;
+		}
+		if (!warned) {
+			GST_DEBUG_OBJECT(self, "Camera configuration adjusted, but no significant changes detected.");
+		}
+		// Update Gst orientation property to match adjusted config
+		self->orientation = libcamera_orientation_to_gst_video_orientation(state->config_->orientation);
+		break;
+	}
+	case CameraConfiguration::Invalid:
+		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
+				  ("Camera configuration is not supported"),
+				  ("CameraConfiguration::validate() returned Invalid"));
 		return false;
+	}
 
 	int ret = state->cam_->configure(state->config_.get());
 	if (ret) {
@@ -926,6 +1031,9 @@  gst_libcamera_src_set_property(GObject *object, guint prop_id,
 		g_free(self->camera_name);
 		self->camera_name = g_value_dup_string(value);
 		break;
+	case PROP_ORIENTATION:
+		self->orientation = (GstVideoOrientationMethod)g_value_get_enum(value);
+		break;
 	default:
 		if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
 			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
@@ -945,6 +1053,9 @@  gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
 	case PROP_CAMERA_NAME:
 		g_value_set_string(value, self->camera_name);
 		break;
+	case PROP_ORIENTATION:
+		g_value_set_enum(value, (gint)self->orientation);
+		break;
 	default:
 		if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
 			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
@@ -1154,6 +1265,17 @@  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
 							     | G_PARAM_STATIC_STRINGS));
 	g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
 
+	/* Register the orientation enum type. */
+	spec = g_param_spec_enum("orientation", "Orientation",
+					       "Select the orientation of the camera.",
+					       GST_TYPE_VIDEO_ORIENTATION_METHOD,
+					       GST_VIDEO_ORIENTATION_IDENTITY,
+					       (GParamFlags)(GST_PARAM_MUTABLE_READY
+							     | G_PARAM_CONSTRUCT
+							     | G_PARAM_READWRITE
+							     | G_PARAM_STATIC_STRINGS));
+	g_object_class_install_property(object_class, PROP_ORIENTATION, spec);
+
 	GstCameraControls::installProperties(object_class, PROP_LAST);
 }