| Message ID | 20250722105627.11961-2-jaslo@ziska.de |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
+Nicolas, Quoting Jaslo Ziska (2025-07-22 11:39:28) > gst_pad_peer_query_accept_caps() might only check if the caps are > acceptable with the peer element, but not recursively with all > downstream elements. If the reconfigure flag was set because the > pipeline downstream changed, gst_pad_peer_query_accept_caps() might still > return true, even though downstream can't handle the current caps, which > causes a not-negotiated error. > > This commit fixes this issue by emitting a query event which > recursively checks with all downstream elements. Because at this point > we are only interested in whether the current caps are still acceptable, > use the currently used caps as a filter and then check if the query > returned empty caps. > > Signed-off-by: Jaslo Ziska <jaslo@ziska.de> > --- > src/gstreamer/gstlibcamerasrc.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > index 3aca4eed..7f4a39ec 100644 > --- a/src/gstreamer/gstlibcamerasrc.cpp > +++ b/src/gstreamer/gstlibcamerasrc.cpp > @@ -730,7 +730,8 @@ gst_libcamera_src_task_run(gpointer user_data) > if (gst_pad_check_reconfigure(srcpad)) { > /* Check if the caps even need changing. */ > g_autoptr(GstCaps) caps = gst_pad_get_current_caps(srcpad); > - if (!gst_pad_peer_query_accept_caps(srcpad, caps)) { > + g_autoptr(GstCaps) peercaps = gst_pad_peer_query_caps(srcpad, caps); > + if (gst_caps_is_empty(peercaps)) { This looks good to me, and tackles Nicolas' previous review comments. Adding Nicolas for awareness of the updated series. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > reconfigure = true; > break; > } > -- > 2.50.0 >
On Tue, Jul 22, 2025 at 12:39:28PM +0200, Jaslo Ziska wrote: > gst_pad_peer_query_accept_caps() might only check if the caps are > acceptable with the peer element, but not recursively with all > downstream elements. If the reconfigure flag was set because the > pipeline downstream changed, gst_pad_peer_query_accept_caps() might still > return true, even though downstream can't handle the current caps, which > causes a not-negotiated error. > > This commit fixes this issue by emitting a query event which > recursively checks with all downstream elements. Because at this point > we are only interested in whether the current caps are still acceptable, > use the currently used caps as a filter and then check if the query > returned empty caps. > nit: Commit message's last line needs to be properly structured. ``` Because at this point we are only interested in checking whether the current caps are still acceptable for downstream elements hence, use the currently configured caps on the source pad as a filter, to check if the gst_pad_peer_query_caps() returned empty caps. ``` > Signed-off-by: Jaslo Ziska <jaslo@ziska.de> Reviewed-by: Umang Jain <uajain@igalia.com> > --- > src/gstreamer/gstlibcamerasrc.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > index 3aca4eed..7f4a39ec 100644 > --- a/src/gstreamer/gstlibcamerasrc.cpp > +++ b/src/gstreamer/gstlibcamerasrc.cpp > @@ -730,7 +730,8 @@ gst_libcamera_src_task_run(gpointer user_data) > if (gst_pad_check_reconfigure(srcpad)) { > /* Check if the caps even need changing. */ > g_autoptr(GstCaps) caps = gst_pad_get_current_caps(srcpad); > - if (!gst_pad_peer_query_accept_caps(srcpad, caps)) { > + g_autoptr(GstCaps) peercaps = gst_pad_peer_query_caps(srcpad, caps); > + if (gst_caps_is_empty(peercaps)) { > reconfigure = true; > break; > } > -- > 2.50.0 >
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 3aca4eed..7f4a39ec 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -730,7 +730,8 @@ gst_libcamera_src_task_run(gpointer user_data) if (gst_pad_check_reconfigure(srcpad)) { /* Check if the caps even need changing. */ g_autoptr(GstCaps) caps = gst_pad_get_current_caps(srcpad); - if (!gst_pad_peer_query_accept_caps(srcpad, caps)) { + g_autoptr(GstCaps) peercaps = gst_pad_peer_query_caps(srcpad, caps); + if (gst_caps_is_empty(peercaps)) { reconfigure = true; break; }
gst_pad_peer_query_accept_caps() might only check if the caps are acceptable with the peer element, but not recursively with all downstream elements. If the reconfigure flag was set because the pipeline downstream changed, gst_pad_peer_query_accept_caps() might still return true, even though downstream can't handle the current caps, which causes a not-negotiated error. This commit fixes this issue by emitting a query event which recursively checks with all downstream elements. Because at this point we are only interested in whether the current caps are still acceptable, use the currently used caps as a filter and then check if the query returned empty caps. Signed-off-by: Jaslo Ziska <jaslo@ziska.de> --- src/gstreamer/gstlibcamerasrc.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)