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

[Kieran Bingham]

Quoting Nicolas Dufresne (2023-11-22 19:21:50)
> 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.

Sorry, it wasn't supposed to be dramatic. A real fix is fine too, but a
revert and re-apply fixed isnt hard either.

This patch was merged without going through our compliler matrix.
On my pc, I can't merge if the compiler matrix fails, but thats not set
up for everyone. I've got four patches now lined up to merge for
Raspberry Pi and blocked. I can wait ... if we have a fix

--
Kieran


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