| Message ID | 20250722105627.11961-3-jaslo@ziska.de |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jaslo Ziska (2025-07-22 11:39:29) > As CameraConfiguration::validate() might alter the configuration > previously negotiated with downstream, log an info when this happens as > GStreamer source elements are not supposed to do that. > > Also check if downstream can accept this new stream configuration and if > not, return a not-negotiated error. > > Signed-off-by: Jaslo Ziska <jaslo@ziska.de> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Aha, I didn't need to add Nicolas to the previous patch :-) This seems like a good addition and verifiaction. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/gstreamer/gstlibcamerasrc.cpp | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > index 7f4a39ec..79a025a5 100644 > --- a/src/gstreamer/gstlibcamerasrc.cpp > +++ b/src/gstreamer/gstlibcamerasrc.cpp > @@ -617,8 +617,13 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self) > } > > /* Validate the configuration. */ > - if (state->config_->validate() == CameraConfiguration::Invalid) > + CameraConfiguration::Status status = state->config_->validate(); > + if (status == CameraConfiguration::Invalid) > return false; > + else if (status == CameraConfiguration::Adjusted) > + GST_ELEMENT_INFO(self, RESOURCE, SETTINGS, > + ("Configuration was adjusted"), > + ("CameraConfiguration::validate() returned CameraConfiguration::Adjusted")); > > int ret = state->cam_->configure(state->config_.get()); > if (ret) { > @@ -643,6 +648,10 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self) > g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]); > gst_libcamera_framerate_to_caps(caps, element_caps); > > + if (status == CameraConfiguration::Adjusted && > + !gst_pad_peer_query_accept_caps(srcpad, caps)) > + return false; > + > if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) > return false; > } > -- > 2.50.0 >
On Tue, Jul 22, 2025 at 12:39:29PM +0200, Jaslo Ziska wrote: > As CameraConfiguration::validate() might alter the configuration > previously negotiated with downstream, log an info when this happens as > GStreamer source elements are not supposed to do that. > > Also check if downstream can accept this new stream configuration and if > not, return a not-negotiated error. > > Signed-off-by: Jaslo Ziska <jaslo@ziska.de> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Reviewed-by: Umang Jain <uajain@igalia.com> > --- > src/gstreamer/gstlibcamerasrc.cpp | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > index 7f4a39ec..79a025a5 100644 > --- a/src/gstreamer/gstlibcamerasrc.cpp > +++ b/src/gstreamer/gstlibcamerasrc.cpp > @@ -617,8 +617,13 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self) > } > > /* Validate the configuration. */ > - if (state->config_->validate() == CameraConfiguration::Invalid) > + CameraConfiguration::Status status = state->config_->validate(); > + if (status == CameraConfiguration::Invalid) > return false; > + else if (status == CameraConfiguration::Adjusted) > + GST_ELEMENT_INFO(self, RESOURCE, SETTINGS, > + ("Configuration was adjusted"), > + ("CameraConfiguration::validate() returned CameraConfiguration::Adjusted")); > > int ret = state->cam_->configure(state->config_.get()); > if (ret) { > @@ -643,6 +648,10 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self) > g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]); > gst_libcamera_framerate_to_caps(caps, element_caps); > > + if (status == CameraConfiguration::Adjusted && > + !gst_pad_peer_query_accept_caps(srcpad, caps)) > + return false; > + > if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) > return false; > } > -- > 2.50.0 >
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 7f4a39ec..79a025a5 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -617,8 +617,13 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self) } /* Validate the configuration. */ - if (state->config_->validate() == CameraConfiguration::Invalid) + CameraConfiguration::Status status = state->config_->validate(); + if (status == CameraConfiguration::Invalid) return false; + else if (status == CameraConfiguration::Adjusted) + GST_ELEMENT_INFO(self, RESOURCE, SETTINGS, + ("Configuration was adjusted"), + ("CameraConfiguration::validate() returned CameraConfiguration::Adjusted")); int ret = state->cam_->configure(state->config_.get()); if (ret) { @@ -643,6 +648,10 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self) g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]); gst_libcamera_framerate_to_caps(caps, element_caps); + if (status == CameraConfiguration::Adjusted && + !gst_pad_peer_query_accept_caps(srcpad, caps)) + return false; + if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) return false; }