[libcamera-devel] [PATCH v3] gstreamer: Implement element EOS handling

[Nicolas Dufresne]

Le mercredi 22 novembre 2023 à 17:24 +0000, Barnabás Pőcze a écrit :
> Hi
> 
> 
> 2023. november 22., szerda 18:08 keltezéssel, Kieran Bingham via libcamera-devel <libcamera-devel at lists.libcamera.org> írta:
> 
> > Quoting Laurent Pinchart via libcamera-devel (2023-11-22 16:38:49)
> > 
> > > Hello,
> > > 
> > > On Tue, Nov 14, 2023 at 01:18:57PM +0100, Jaslo Ziska via libcamera-devel wrote:
> > > 
> > > > This commit implements EOS handling for events sent to the libcamerasrc
> > > > element by the send_event method (which can happen when pressing
> > > > Ctrl-C while running gst-launch-1.0 -e, see below). EOS events from
> > > > downstream elements returning GST_FLOW_EOS are not considered here.
> > > > 
> > > > To archive this add a function for the send_event method which handles
> > > > the GST_EVENT_EOS event. This function will set an atomic to the
> > > > received event and push this EOS event to all source pads in the running
> > > > task.
> > > > 
> > > > Also set the GST_ELEMENT_FLAG_SOURCE flag to identify libcamerasrc as a
> > > > source element which enables it to receive EOS events sent to the
> > > > (pipeline) bin containing it.
> > > > This in turn enables libcamerasrc to for example receive EOS events from
> > > > gst-launch-1.0 when using the -e flag.
> > > > 
> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=91
> > > > Signed-off-by: Jaslo Ziska jaslo at ziska.de
> > > > Acked-by: Nicolas Dufresne nicolas.dufresne at collabora.com
> > > > ---
> > 
> > <snip>
> > 
> > > > +static gboolean
> > > > +gst_libcamera_src_send_event(GstElement *element, GstEvent *event)
> > > > +{
> > > > + GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);
> > > > + gboolean ret = FALSE;
> > > > +
> > > > + switch (GST_EVENT_TYPE(event)) {
> > > > + case GST_EVENT_EOS: {
> > > > + g_autoptr(GstEvent) oldEvent = self->pending_eos.exchange(event);
> > > 
> > > I'm afraid this causes a compilation breakage with clang :-(
> > > 
> > > ../../src/gstreamer/gstlibcamerasrc.cpp:768:23: error: unused variable 'oldEvent' [-Werror,-Wunused-variable]
> > > g_autoptr(GstEvent) oldEvent = self->pending_eos.exchange(event);
> > > ^
> > > 
> > > The patch has been merged, can you propose a fix quickly, or should I
> > > revert to give you more time ?
> > 
> > 
> > I'm afraid with a broken build we're blocked on merging more patches -
> > of which I'm now trying to do - so I'd probably ask if we can revert
> > this until it's fixed please.
> > [...]
> 
> [[maybe_unused]] g_autoptr(GstEvent) oldEvent = self->pending_eos.exchange(event);

Or just, like the original patch did.

   GstEvent *oldEvent = self->pending_eos.exchange(event);
   gst_clear_event(&oldEvent);

Not sure why this blocks anything, sounds a bit dramatic handling of a build
warning. In this case, its quite obvious that this is a false positive in clang
warning machinery, you will hit more of these with clang in my experience.

Nicolas

> 
> ?
> 
> > [...]
> 
> Regards,
> Barnabás Pőcze