[libcamera-devel,v2] gstreamer: Fix unused variable error
diff mbox series

Message ID 20231123075736.3884-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 62e5289819789e37cb169af94869fea0c86c8e9c
Headers show
Series
  • [libcamera-devel,v2] gstreamer: Fix unused variable error
Related show

Commit Message

Laurent Pinchart Nov. 23, 2023, 7:57 a.m. UTC 
From: Jaslo Ziska <jaslo@ziska.de>

Commit fd84180d7a09 ("gstreamer: Implement element EOS handling") has
introduced a compilation warning with clang:

../../src/gstreamer/gstlibcamerasrc.cpp:768:23: error: unused variable 'oldEvent' [-Werror,-Wunused-variable]
                g_autoptr(GstEvent) oldEvent = self->pending_eos.exchange(event);
                                    ^

This seems to be a false positive, but nonetheless breaks the build. Fix
it.

Fixes: fd84180d7a09 ("gstreamer: Implement element EOS handling")
Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/gstreamer/gstlibcamerasrc.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 2fae9603e6cc483d9d0d74868721b272776513cf

Comments

Umang Jain Nov. 23, 2023, 7:58 a.m. UTC | #1 
Hi Laurent,

Thank you for handling this.

On 11/23/23 1:27 PM, Laurent Pinchart via libcamera-devel wrote:
> From: Jaslo Ziska <jaslo@ziska.de>
>
> Commit fd84180d7a09 ("gstreamer: Implement element EOS handling") has
> introduced a compilation warning with clang:
>
> ../../src/gstreamer/gstlibcamerasrc.cpp:768:23: error: unused variable 'oldEvent' [-Werror,-Wunused-variable]
>                  g_autoptr(GstEvent) oldEvent = self->pending_eos.exchange(event);
>                                      ^
>
> This seems to be a false positive, but nonetheless breaks the build. Fix
> it.
>
> Fixes: fd84180d7a09 ("gstreamer: Implement element EOS handling")
> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   src/gstreamer/gstlibcamerasrc.cpp | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 767017db63f5..a6f240f56873 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -765,8 +765,8 @@ gst_libcamera_src_send_event(GstElement *element, GstEvent *event)
>   
>   	switch (GST_EVENT_TYPE(event)) {
>   	case GST_EVENT_EOS: {
> -		g_autoptr(GstEvent) oldEvent = self->pending_eos.exchange(event);
> -
> +		GstEvent *oldEvent = self->pending_eos.exchange(event);
> +		gst_clear_event(&oldEvent);
>   		ret = TRUE;
>   		break;
>   	}
>
> base-commit: 2fae9603e6cc483d9d0d74868721b272776513cf
Jaslo Ziska Nov. 23, 2023, 10:53 a.m. UTC | #2 
Hi Laurent,

thank you very much for taking care of this.

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> From: Jaslo Ziska <jaslo@ziska.de>
>
> Commit fd84180d7a09 ("gstreamer: Implement element EOS 
> handling") has
> introduced a compilation warning with clang:
>
> ../../src/gstreamer/gstlibcamerasrc.cpp:768:23: error: unused 
> variable 'oldEvent' [-Werror,-Wunused-variable]
>                 g_autoptr(GstEvent) oldEvent = 
>                 self->pending_eos.exchange(event);
>                                     ^
>
> This seems to be a false positive, but nonetheless breaks the 
> build. Fix
> it.
>
> Fixes: fd84180d7a09 ("gstreamer: Implement element EOS 
> handling")
> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> Signed-off-by: Laurent Pinchart 
> <laurent.pinchart@ideasonboard.com>
> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp 
> b/src/gstreamer/gstlibcamerasrc.cpp
> index 767017db63f5..a6f240f56873 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -765,8 +765,8 @@ gst_libcamera_src_send_event(GstElement 
> *element, GstEvent *event)
>
>  	switch (GST_EVENT_TYPE(event)) {
>  	case GST_EVENT_EOS: {
> -		g_autoptr(GstEvent) oldEvent = 
> self->pending_eos.exchange(event);
> -
> +		GstEvent *oldEvent = self->pending_eos.exchange(event);
> +		gst_clear_event(&oldEvent);
>  		ret = TRUE;
>  		break;
>  	}
>
> base-commit: 2fae9603e6cc483d9d0d74868721b272776513cf

The changes look good. I really don't need credit for this, its 
such a minor change and I did not contribute much.

Who would have thought that my first patch leads to such a small 
chaos ;)

Regards,

Jaslo
Kieran Bingham Nov. 23, 2023, 11:06 a.m. UTC | #3 
Quoting Jaslo Ziska via libcamera-devel (2023-11-23 10:53:09)
> Hi Laurent,
> 
> thank you very much for taking care of this.
> 
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> > From: Jaslo Ziska <jaslo@ziska.de>
> >
> > Commit fd84180d7a09 ("gstreamer: Implement element EOS 
> > handling") has
> > introduced a compilation warning with clang:
> >
> > ../../src/gstreamer/gstlibcamerasrc.cpp:768:23: error: unused 
> > variable 'oldEvent' [-Werror,-Wunused-variable]
> >                 g_autoptr(GstEvent) oldEvent = 
> >                 self->pending_eos.exchange(event);
> >                                     ^
> >
> > This seems to be a false positive, but nonetheless breaks the 
> > build. Fix
> > it.
> >
> > Fixes: fd84180d7a09 ("gstreamer: Implement element EOS 
> > handling")
> > Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> > Signed-off-by: Laurent Pinchart 
> > <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/gstreamer/gstlibcamerasrc.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp 
> > b/src/gstreamer/gstlibcamerasrc.cpp
> > index 767017db63f5..a6f240f56873 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -765,8 +765,8 @@ gst_libcamera_src_send_event(GstElement 
> > *element, GstEvent *event)
> >
> >       switch (GST_EVENT_TYPE(event)) {
> >       case GST_EVENT_EOS: {
> > -             g_autoptr(GstEvent) oldEvent = 
> > self->pending_eos.exchange(event);
> > -
> > +             GstEvent *oldEvent = self->pending_eos.exchange(event);
> > +             gst_clear_event(&oldEvent);
> >               ret = TRUE;
> >               break;
> >       }
> >
> > base-commit: 2fae9603e6cc483d9d0d74868721b272776513cf
> 
> The changes look good. I really don't need credit for this, its 
> such a minor change and I did not contribute much.
> 
> Who would have thought that my first patch leads to such a small 
> chaos ;)

That's no one's fault except ours for not already having a formal method
in place to run the whole pre-merge integration tests easily.

Thanks for your everyones contributions to getting this fixed!

--
Regards


Kieran

> 
> Regards,
> 
> Jaslo
Laurent Pinchart Nov. 23, 2023, 12:20 p.m. UTC | #4 
Hi Jaslo,

On Thu, Nov 23, 2023 at 11:53:09AM +0100, Jaslo Ziska wrote:
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> > From: Jaslo Ziska <jaslo@ziska.de>
> >
> > Commit fd84180d7a09 ("gstreamer: Implement element EOS 
> > handling") has
> > introduced a compilation warning with clang:
> >
> > ../../src/gstreamer/gstlibcamerasrc.cpp:768:23: error: unused 
> > variable 'oldEvent' [-Werror,-Wunused-variable]
> >                 g_autoptr(GstEvent) oldEvent = 
> >                 self->pending_eos.exchange(event);
> >                                     ^
> >
> > This seems to be a false positive, but nonetheless breaks the 
> > build. Fix
> > it.
> >
> > Fixes: fd84180d7a09 ("gstreamer: Implement element EOS 
> > handling")
> > Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> > Signed-off-by: Laurent Pinchart 
> > <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/gstreamer/gstlibcamerasrc.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp 
> > b/src/gstreamer/gstlibcamerasrc.cpp
> > index 767017db63f5..a6f240f56873 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -765,8 +765,8 @@ gst_libcamera_src_send_event(GstElement 
> > *element, GstEvent *event)
> >
> >  	switch (GST_EVENT_TYPE(event)) {
> >  	case GST_EVENT_EOS: {
> > -		g_autoptr(GstEvent) oldEvent = 
> > self->pending_eos.exchange(event);
> > -
> > +		GstEvent *oldEvent = self->pending_eos.exchange(event);
> > +		gst_clear_event(&oldEvent);
> >  		ret = TRUE;
> >  		break;
> >  	}
> >
> > base-commit: 2fae9603e6cc483d9d0d74868721b272776513cf
> 
> The changes look good. I really don't need credit for this, its 
> such a minor change and I did not contribute much.

Minor or not, you submitted the patch, so you deserve credit for it.
That's the least we can do (as well as treating you and everybody as
nicely as we can).

> Who would have thought that my first patch leads to such a small 
> chaos ;)

I'm sorry it looked like chaos, Kieran, Umang and I didn't communicate
properly on this. At the end of the day, a breakage, a possible revert,
and a fix are things that happen routinely, you didn't do anything bad.

Patch
diff mbox series 

diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 767017db63f5..a6f240f56873 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -765,8 +765,8 @@  gst_libcamera_src_send_event(GstElement *element, GstEvent *event)
 
 	switch (GST_EVENT_TYPE(event)) {
 	case GST_EVENT_EOS: {
-		g_autoptr(GstEvent) oldEvent = self->pending_eos.exchange(event);
-
+		GstEvent *oldEvent = self->pending_eos.exchange(event);
+		gst_clear_event(&oldEvent);
 		ret = TRUE;
 		break;
 	}