[libcamera-devel,RFC,v2] gstreamer: Factor out _pad_push_stream_start from _task_enter
diff mbox series

Message ID 20210801185144.21572-1-vedantparanjape160201@gmail.com
State New
Headers show
Series
  • [libcamera-devel,RFC,v2] gstreamer: Factor out _pad_push_stream_start from _task_enter
Related show

Commit Message

Vedant Paranjape Aug. 1, 2021, 6:51 p.m. UTC
This patch creates gst_libcamera_pad_push_stream_start function to
create stream id and to push the stream start.

Update functional code in gst_libcamera_src_task_enter(), which creates
stream id and pushes the stream start with the refactored
function gst_libcamera_pad_push_stream_start().

Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
---
 src/gstreamer/gstlibcamerapad.cpp | 19 +++++++++++++++++++
 src/gstreamer/gstlibcamerapad.h   |  2 ++
 src/gstreamer/gstlibcamerasrc.cpp |  7 +------
 3 files changed, 22 insertions(+), 6 deletions(-)

Comments

Paul Elder Aug. 2, 2021, 4:46 a.m. UTC | #1
Hi Vedant,

On Mon, Aug 02, 2021 at 12:21:44AM +0530, Vedant Paranjape wrote:
> This patch creates gst_libcamera_pad_push_stream_start function to
> create stream id and to push the stream start.
> 
> Update functional code in gst_libcamera_src_task_enter(), which creates
> stream id and pushes the stream start with the refactored
> function gst_libcamera_pad_push_stream_start().

The code looks better than before, but I'm not quite seeing the
rationale. Why is this change important? Perhaps you told me before, but
the "why" is what should be in the commit message, so that reviewers can
see. To some extent, the "what" isn't as important, since the reviewer
can simply read the patch (though it is useful when the patch is long
and complex).


Paul

> 
> Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> ---
>  src/gstreamer/gstlibcamerapad.cpp | 19 +++++++++++++++++++
>  src/gstreamer/gstlibcamerapad.h   |  2 ++
>  src/gstreamer/gstlibcamerasrc.cpp |  7 +------
>  3 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp
> index c00e81c8..d4211050 100644
> --- a/src/gstreamer/gstlibcamerapad.cpp
> +++ b/src/gstreamer/gstlibcamerapad.cpp
> @@ -20,6 +20,7 @@ struct _GstLibcameraPad {
>  	GstLibcameraPool *pool;
>  	GQueue pending_buffers;
>  	GstClockTime latency;
> +	gint stream_id_num;
>  };
>  
>  enum {
> @@ -81,6 +82,7 @@ static void
>  gst_libcamera_pad_init(GstLibcameraPad *self)
>  {
>  	GST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;
> +	self->stream_id_num = 0;
>  }
>  
>  static GType
> @@ -155,6 +157,23 @@ gst_libcamera_pad_get_stream(GstPad *pad)
>  	return nullptr;
>  }
>  
> +void
> +gst_libcamera_pad_push_stream_start(GstPad *pad, const guint group_id)
> +{
> +	auto *self = GST_LIBCAMERA_PAD(pad);	
> +	{
> +		GLibLocker lock(GST_OBJECT(self));
> +		self->stream_id_num++;
> +	}
> +
> +	g_autoptr(GstElement) element = gst_pad_get_parent_element(pad);
> +	g_autofree gchar *stream_id_intermediate = g_strdup_printf("%i%i", group_id, self->stream_id_num);
> +	g_autofree gchar *stream_id = gst_pad_create_stream_id(pad, element, stream_id_intermediate);
> +	GstEvent *event = gst_event_new_stream_start(stream_id);
> +	gst_event_set_group_id(event, group_id);
> +	gst_pad_push_event(pad, event);
> +}
> +
>  void
>  gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer)
>  {
> diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h
> index 779f2d13..7693374f 100644
> --- a/src/gstreamer/gstlibcamerapad.h
> +++ b/src/gstreamer/gstlibcamerapad.h
> @@ -26,6 +26,8 @@ void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool);
>  
>  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);
>  
> +void gst_libcamera_pad_push_stream_start(GstPad *pad, const guint group_id);
> +
>  void gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer);
>  
>  GstFlowReturn gst_libcamera_pad_push_pending(GstPad *pad);
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index b18f1efb..bb8ea07a 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -361,15 +361,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
>  
>  	GST_DEBUG_OBJECT(self, "Streaming thread has started");
>  
> -	gint stream_id_num = 0;
>  	StreamRoles roles;
>  	for (GstPad *srcpad : state->srcpads_) {
>  		/* Create stream-id and push stream-start. */
> -		g_autofree gchar *stream_id_intermediate = g_strdup_printf("%i%i", state->group_id_, stream_id_num++);
> -		g_autofree gchar *stream_id = gst_pad_create_stream_id(srcpad, GST_ELEMENT(self), stream_id_intermediate);
> -		GstEvent *event = gst_event_new_stream_start(stream_id);
> -		gst_event_set_group_id(event, state->group_id_);
> -		gst_pad_push_event(srcpad, event);
> +		gst_libcamera_pad_push_stream_start(srcpad, state->group_id_);
>  
>  		/* Collect the streams roles for the next iteration. */
>  		roles.push_back(gst_libcamera_pad_get_role(srcpad));
> -- 
> 2.25.1
>
Vedant Paranjape Aug. 2, 2021, 5:56 a.m. UTC | #2
Hi Paul,
This change is needed as the specific code is pad specific and pretty
generic. Also it might be used again in reconfig function. So to reduce
code duplication, this is needed. ood?

I will add this explaination in commit message, does it sound good to you?

Regards,
Vedant Paranjape

On Mon, 2 Aug, 2021, 10:16 , <paul.elder@ideasonboard.com> wrote:

> Hi Vedant,
>
> On Mon, Aug 02, 2021 at 12:21:44AM +0530, Vedant Paranjape wrote:
> > This patch creates gst_libcamera_pad_push_stream_start function to
> > create stream id and to push the stream start.
> >
> > Update functional code in gst_libcamera_src_task_enter(), which creates
> > stream id and pushes the stream start with the refactored
> > function gst_libcamera_pad_push_stream_start().
>
> The code looks better than before, but I'm not quite seeing the
> rationale. Why is this change important? Perhaps you told me before, but
> the "why" is what should be in the commit message, so that reviewers can
> see. To some extent, the "what" isn't as important, since the reviewer
> can simply read the patch (though it is useful when the patch is long
> and complex).
>
>
> Paul
>
> >
> > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> > ---
> >  src/gstreamer/gstlibcamerapad.cpp | 19 +++++++++++++++++++
> >  src/gstreamer/gstlibcamerapad.h   |  2 ++
> >  src/gstreamer/gstlibcamerasrc.cpp |  7 +------
> >  3 files changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/gstreamer/gstlibcamerapad.cpp
> b/src/gstreamer/gstlibcamerapad.cpp
> > index c00e81c8..d4211050 100644
> > --- a/src/gstreamer/gstlibcamerapad.cpp
> > +++ b/src/gstreamer/gstlibcamerapad.cpp
> > @@ -20,6 +20,7 @@ struct _GstLibcameraPad {
> >       GstLibcameraPool *pool;
> >       GQueue pending_buffers;
> >       GstClockTime latency;
> > +     gint stream_id_num;
> >  };
> >
> >  enum {
> > @@ -81,6 +82,7 @@ static void
> >  gst_libcamera_pad_init(GstLibcameraPad *self)
> >  {
> >       GST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;
> > +     self->stream_id_num = 0;
> >  }
> >
> >  static GType
> > @@ -155,6 +157,23 @@ gst_libcamera_pad_get_stream(GstPad *pad)
> >       return nullptr;
> >  }
> >
> > +void
> > +gst_libcamera_pad_push_stream_start(GstPad *pad, const guint group_id)
> > +{
> > +     auto *self = GST_LIBCAMERA_PAD(pad);
> > +     {
> > +             GLibLocker lock(GST_OBJECT(self));
> > +             self->stream_id_num++;
> > +     }
> > +
> > +     g_autoptr(GstElement) element = gst_pad_get_parent_element(pad);
> > +     g_autofree gchar *stream_id_intermediate = g_strdup_printf("%i%i",
> group_id, self->stream_id_num);
> > +     g_autofree gchar *stream_id = gst_pad_create_stream_id(pad,
> element, stream_id_intermediate);
> > +     GstEvent *event = gst_event_new_stream_start(stream_id);
> > +     gst_event_set_group_id(event, group_id);
> > +     gst_pad_push_event(pad, event);
> > +}
> > +
> >  void
> >  gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer)
> >  {
> > diff --git a/src/gstreamer/gstlibcamerapad.h
> b/src/gstreamer/gstlibcamerapad.h
> > index 779f2d13..7693374f 100644
> > --- a/src/gstreamer/gstlibcamerapad.h
> > +++ b/src/gstreamer/gstlibcamerapad.h
> > @@ -26,6 +26,8 @@ void gst_libcamera_pad_set_pool(GstPad *pad,
> GstLibcameraPool *pool);
> >
> >  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);
> >
> > +void gst_libcamera_pad_push_stream_start(GstPad *pad, const guint
> group_id);
> > +
> >  void gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer);
> >
> >  GstFlowReturn gst_libcamera_pad_push_pending(GstPad *pad);
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> b/src/gstreamer/gstlibcamerasrc.cpp
> > index b18f1efb..bb8ea07a 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -361,15 +361,10 @@ gst_libcamera_src_task_enter(GstTask *task,
> [[maybe_unused]] GThread *thread,
> >
> >       GST_DEBUG_OBJECT(self, "Streaming thread has started");
> >
> > -     gint stream_id_num = 0;
> >       StreamRoles roles;
> >       for (GstPad *srcpad : state->srcpads_) {
> >               /* Create stream-id and push stream-start. */
> > -             g_autofree gchar *stream_id_intermediate =
> g_strdup_printf("%i%i", state->group_id_, stream_id_num++);
> > -             g_autofree gchar *stream_id =
> gst_pad_create_stream_id(srcpad, GST_ELEMENT(self), stream_id_intermediate);
> > -             GstEvent *event = gst_event_new_stream_start(stream_id);
> > -             gst_event_set_group_id(event, state->group_id_);
> > -             gst_pad_push_event(srcpad, event);
> > +             gst_libcamera_pad_push_stream_start(srcpad,
> state->group_id_);
> >
> >               /* Collect the streams roles for the next iteration. */
> >               roles.push_back(gst_libcamera_pad_get_role(srcpad));
> > --
> > 2.25.1
> >
>
Paul Elder Aug. 2, 2021, 6:56 a.m. UTC | #3
Hi Vedant,

On Mon, Aug 02, 2021 at 11:26:56AM +0530, Vedant Paranjape wrote:
> Hi Paul,
> This change is needed as the specific code is pad specific and pretty generic.

"pad-specific" and "generic"? :D

As in, the code is specific to pads (so it belongs in pad and not src),
and generic as a function for pads?

> Also it might be used again in reconfig function. So to reduce code

s/might/will ?

> duplication, this is needed. ood?
> 
> I will add this explaination in commit message, does it sound good to you?

iirc you also said there was some significance in changing the stream
id. Explaining that in the commit message would be good too.


Paul

> 
> On Mon, 2 Aug, 2021, 10:16 , <paul.elder@ideasonboard.com> wrote:
> 
>     Hi Vedant,
> 
>     On Mon, Aug 02, 2021 at 12:21:44AM +0530, Vedant Paranjape wrote:
>     > This patch creates gst_libcamera_pad_push_stream_start function to
>     > create stream id and to push the stream start.
>     >
>     > Update functional code in gst_libcamera_src_task_enter(), which creates
>     > stream id and pushes the stream start with the refactored
>     > function gst_libcamera_pad_push_stream_start().
> 
>     The code looks better than before, but I'm not quite seeing the
>     rationale. Why is this change important? Perhaps you told me before, but
>     the "why" is what should be in the commit message, so that reviewers can
>     see. To some extent, the "what" isn't as important, since the reviewer
>     can simply read the patch (though it is useful when the patch is long
>     and complex).
> 
> 
>     Paul
> 
>     >
>     > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
>     > ---
>     >  src/gstreamer/gstlibcamerapad.cpp | 19 +++++++++++++++++++
>     >  src/gstreamer/gstlibcamerapad.h   |  2 ++
>     >  src/gstreamer/gstlibcamerasrc.cpp |  7 +------
>     >  3 files changed, 22 insertions(+), 6 deletions(-)
>     >
>     > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/
>     gstlibcamerapad.cpp
>     > index c00e81c8..d4211050 100644
>     > --- a/src/gstreamer/gstlibcamerapad.cpp
>     > +++ b/src/gstreamer/gstlibcamerapad.cpp
>     > @@ -20,6 +20,7 @@ struct _GstLibcameraPad {
>     >       GstLibcameraPool *pool;
>     >       GQueue pending_buffers;
>     >       GstClockTime latency;
>     > +     gint stream_id_num;
>     >  };
>     > 
>     >  enum {
>     > @@ -81,6 +82,7 @@ static void
>     >  gst_libcamera_pad_init(GstLibcameraPad *self)
>     >  {
>     >       GST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;
>     > +     self->stream_id_num = 0;
>     >  }
>     > 
>     >  static GType
>     > @@ -155,6 +157,23 @@ gst_libcamera_pad_get_stream(GstPad *pad)
>     >       return nullptr;
>     >  }
>     > 
>     > +void
>     > +gst_libcamera_pad_push_stream_start(GstPad *pad, const guint group_id)
>     > +{
>     > +     auto *self = GST_LIBCAMERA_PAD(pad);   
>     > +     {
>     > +             GLibLocker lock(GST_OBJECT(self));
>     > +             self->stream_id_num++;
>     > +     }
>     > +
>     > +     g_autoptr(GstElement) element = gst_pad_get_parent_element(pad);
>     > +     g_autofree gchar *stream_id_intermediate = g_strdup_printf("%i%i",
>     group_id, self->stream_id_num);
>     > +     g_autofree gchar *stream_id = gst_pad_create_stream_id(pad,
>     element, stream_id_intermediate);
>     > +     GstEvent *event = gst_event_new_stream_start(stream_id);
>     > +     gst_event_set_group_id(event, group_id);
>     > +     gst_pad_push_event(pad, event);
>     > +}
>     > +
>     >  void
>     >  gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer)
>     >  {
>     > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/
>     gstlibcamerapad.h
>     > index 779f2d13..7693374f 100644
>     > --- a/src/gstreamer/gstlibcamerapad.h
>     > +++ b/src/gstreamer/gstlibcamerapad.h
>     > @@ -26,6 +26,8 @@ void gst_libcamera_pad_set_pool(GstPad *pad,
>     GstLibcameraPool *pool);
>     > 
>     >  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);
>     > 
>     > +void gst_libcamera_pad_push_stream_start(GstPad *pad, const guint
>     group_id);
>     > +
>     >  void gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer);
>     > 
>     >  GstFlowReturn gst_libcamera_pad_push_pending(GstPad *pad);
>     > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/
>     gstlibcamerasrc.cpp
>     > index b18f1efb..bb8ea07a 100644
>     > --- a/src/gstreamer/gstlibcamerasrc.cpp
>     > +++ b/src/gstreamer/gstlibcamerasrc.cpp
>     > @@ -361,15 +361,10 @@ gst_libcamera_src_task_enter(GstTask *task,
>     [[maybe_unused]] GThread *thread,
>     > 
>     >       GST_DEBUG_OBJECT(self, "Streaming thread has started");
>     > 
>     > -     gint stream_id_num = 0;
>     >       StreamRoles roles;
>     >       for (GstPad *srcpad : state->srcpads_) {
>     >               /* Create stream-id and push stream-start. */
>     > -             g_autofree gchar *stream_id_intermediate = g_strdup_printf
>     ("%i%i", state->group_id_, stream_id_num++);
>     > -             g_autofree gchar *stream_id = gst_pad_create_stream_id
>     (srcpad, GST_ELEMENT(self), stream_id_intermediate);
>     > -             GstEvent *event = gst_event_new_stream_start(stream_id);
>     > -             gst_event_set_group_id(event, state->group_id_);
>     > -             gst_pad_push_event(srcpad, event);
>     > +             gst_libcamera_pad_push_stream_start(srcpad, state->
>     group_id_);
>     > 
>     >               /* Collect the streams roles for the next iteration. */
>     >               roles.push_back(gst_libcamera_pad_get_role(srcpad));
>     > --
>     > 2.25.1
>     >
>
Vedant Paranjape Aug. 2, 2021, 7:15 a.m. UTC | #4
Hi Paul

On Mon, 2 Aug, 2021, 12:26 , <paul.elder@ideasonboard.com> wrote:

> Hi Vedant,
>
> On Mon, Aug 02, 2021 at 11:26:56AM +0530, Vedant Paranjape wrote:
> > Hi Paul,
> > This change is needed as the specific code is pad specific and pretty
> generic.
>
> "pad-specific" and "generic"? :D


Oops, I meant the "the pad-specific code" is used in several places and not
libcamera specific.


> As in, the code is specific to pads (so it belongs in pad and not src),
> and generic as a function for pads?
>

Yes, it should belong to the pads. Generic as in "typical gstreamer code
for pads", I meant to say in the sense that it can be reused in lot of
places again.


> > Also it might be used again in reconfig function. So to reduce code
>
> s/might/will ?
>
> > duplication, this is needed. ood?
> >
> > I will add this explaination in commit message, does it sound good to
> you?
>
> iirc you also said there was some significance in changing the stream
> id. Explaining that in the commit message would be good too.
>

Yup, the one where I said, "stream id is something unique and should stay
different for all streams during lifetime of a application", right ?

Regards
Vedant

Paul
>
> >
> > On Mon, 2 Aug, 2021, 10:16 , <paul.elder@ideasonboard.com> wrote:
> >
> >     Hi Vedant,
> >
> >     On Mon, Aug 02, 2021 at 12:21:44AM +0530, Vedant Paranjape wrote:
> >     > This patch creates gst_libcamera_pad_push_stream_start function to
> >     > create stream id and to push the stream start.
> >     >
> >     > Update functional code in gst_libcamera_src_task_enter(), which
> creates
> >     > stream id and pushes the stream start with the refactored
> >     > function gst_libcamera_pad_push_stream_start().
> >
> >     The code looks better than before, but I'm not quite seeing the
> >     rationale. Why is this change important? Perhaps you told me before,
> but
> >     the "why" is what should be in the commit message, so that reviewers
> can
> >     see. To some extent, the "what" isn't as important, since the
> reviewer
> >     can simply read the patch (though it is useful when the patch is long
> >     and complex).
> >
> >
> >     Paul
> >
> >     >
> >     > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> >     > ---
> >     >  src/gstreamer/gstlibcamerapad.cpp | 19 +++++++++++++++++++
> >     >  src/gstreamer/gstlibcamerapad.h   |  2 ++
> >     >  src/gstreamer/gstlibcamerasrc.cpp |  7 +------
> >     >  3 files changed, 22 insertions(+), 6 deletions(-)
> >     >
> >     > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/
> >     gstlibcamerapad.cpp
> >     > index c00e81c8..d4211050 100644
> >     > --- a/src/gstreamer/gstlibcamerapad.cpp
> >     > +++ b/src/gstreamer/gstlibcamerapad.cpp
> >     > @@ -20,6 +20,7 @@ struct _GstLibcameraPad {
> >     >       GstLibcameraPool *pool;
> >     >       GQueue pending_buffers;
> >     >       GstClockTime latency;
> >     > +     gint stream_id_num;
> >     >  };
> >     >
> >     >  enum {
> >     > @@ -81,6 +82,7 @@ static void
> >     >  gst_libcamera_pad_init(GstLibcameraPad *self)
> >     >  {
> >     >       GST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;
> >     > +     self->stream_id_num = 0;
> >     >  }
> >     >
> >     >  static GType
> >     > @@ -155,6 +157,23 @@ gst_libcamera_pad_get_stream(GstPad *pad)
> >     >       return nullptr;
> >     >  }
> >     >
> >     > +void
> >     > +gst_libcamera_pad_push_stream_start(GstPad *pad, const guint
> group_id)
> >     > +{
> >     > +     auto *self = GST_LIBCAMERA_PAD(pad);
> >     > +     {
> >     > +             GLibLocker lock(GST_OBJECT(self));
> >     > +             self->stream_id_num++;
> >     > +     }
> >     > +
> >     > +     g_autoptr(GstElement) element =
> gst_pad_get_parent_element(pad);
> >     > +     g_autofree gchar *stream_id_intermediate =
> g_strdup_printf("%i%i",
> >     group_id, self->stream_id_num);
> >     > +     g_autofree gchar *stream_id = gst_pad_create_stream_id(pad,
> >     element, stream_id_intermediate);
> >     > +     GstEvent *event = gst_event_new_stream_start(stream_id);
> >     > +     gst_event_set_group_id(event, group_id);
> >     > +     gst_pad_push_event(pad, event);
> >     > +}
> >     > +
> >     >  void
> >     >  gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer)
> >     >  {
> >     > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/
> >     gstlibcamerapad.h
> >     > index 779f2d13..7693374f 100644
> >     > --- a/src/gstreamer/gstlibcamerapad.h
> >     > +++ b/src/gstreamer/gstlibcamerapad.h
> >     > @@ -26,6 +26,8 @@ void gst_libcamera_pad_set_pool(GstPad *pad,
> >     GstLibcameraPool *pool);
> >     >
> >     >  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);
> >     >
> >     > +void gst_libcamera_pad_push_stream_start(GstPad *pad, const guint
> >     group_id);
> >     > +
> >     >  void gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer
> *buffer);
> >     >
> >     >  GstFlowReturn gst_libcamera_pad_push_pending(GstPad *pad);
> >     > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/
> >     gstlibcamerasrc.cpp
> >     > index b18f1efb..bb8ea07a 100644
> >     > --- a/src/gstreamer/gstlibcamerasrc.cpp
> >     > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> >     > @@ -361,15 +361,10 @@ gst_libcamera_src_task_enter(GstTask *task,
> >     [[maybe_unused]] GThread *thread,
> >     >
> >     >       GST_DEBUG_OBJECT(self, "Streaming thread has started");
> >     >
> >     > -     gint stream_id_num = 0;
> >     >       StreamRoles roles;
> >     >       for (GstPad *srcpad : state->srcpads_) {
> >     >               /* Create stream-id and push stream-start. */
> >     > -             g_autofree gchar *stream_id_intermediate =
> g_strdup_printf
> >     ("%i%i", state->group_id_, stream_id_num++);
> >     > -             g_autofree gchar *stream_id =
> gst_pad_create_stream_id
> >     (srcpad, GST_ELEMENT(self), stream_id_intermediate);
> >     > -             GstEvent *event =
> gst_event_new_stream_start(stream_id);
> >     > -             gst_event_set_group_id(event, state->group_id_);
> >     > -             gst_pad_push_event(srcpad, event);
> >     > +             gst_libcamera_pad_push_stream_start(srcpad, state->
> >     group_id_);
> >     >
> >     >               /* Collect the streams roles for the next iteration.
> */
> >     >               roles.push_back(gst_libcamera_pad_get_role(srcpad));
> >     > --
> >     > 2.25.1
> >     >
> >
>

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp
index c00e81c8..d4211050 100644
--- a/src/gstreamer/gstlibcamerapad.cpp
+++ b/src/gstreamer/gstlibcamerapad.cpp
@@ -20,6 +20,7 @@  struct _GstLibcameraPad {
 	GstLibcameraPool *pool;
 	GQueue pending_buffers;
 	GstClockTime latency;
+	gint stream_id_num;
 };
 
 enum {
@@ -81,6 +82,7 @@  static void
 gst_libcamera_pad_init(GstLibcameraPad *self)
 {
 	GST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;
+	self->stream_id_num = 0;
 }
 
 static GType
@@ -155,6 +157,23 @@  gst_libcamera_pad_get_stream(GstPad *pad)
 	return nullptr;
 }
 
+void
+gst_libcamera_pad_push_stream_start(GstPad *pad, const guint group_id)
+{
+	auto *self = GST_LIBCAMERA_PAD(pad);	
+	{
+		GLibLocker lock(GST_OBJECT(self));
+		self->stream_id_num++;
+	}
+
+	g_autoptr(GstElement) element = gst_pad_get_parent_element(pad);
+	g_autofree gchar *stream_id_intermediate = g_strdup_printf("%i%i", group_id, self->stream_id_num);
+	g_autofree gchar *stream_id = gst_pad_create_stream_id(pad, element, stream_id_intermediate);
+	GstEvent *event = gst_event_new_stream_start(stream_id);
+	gst_event_set_group_id(event, group_id);
+	gst_pad_push_event(pad, event);
+}
+
 void
 gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer)
 {
diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h
index 779f2d13..7693374f 100644
--- a/src/gstreamer/gstlibcamerapad.h
+++ b/src/gstreamer/gstlibcamerapad.h
@@ -26,6 +26,8 @@  void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool);
 
 libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);
 
+void gst_libcamera_pad_push_stream_start(GstPad *pad, const guint group_id);
+
 void gst_libcamera_pad_queue_buffer(GstPad *pad, GstBuffer *buffer);
 
 GstFlowReturn gst_libcamera_pad_push_pending(GstPad *pad);
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index b18f1efb..bb8ea07a 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -361,15 +361,10 @@  gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
 
 	GST_DEBUG_OBJECT(self, "Streaming thread has started");
 
-	gint stream_id_num = 0;
 	StreamRoles roles;
 	for (GstPad *srcpad : state->srcpads_) {
 		/* Create stream-id and push stream-start. */
-		g_autofree gchar *stream_id_intermediate = g_strdup_printf("%i%i", state->group_id_, stream_id_num++);
-		g_autofree gchar *stream_id = gst_pad_create_stream_id(srcpad, GST_ELEMENT(self), stream_id_intermediate);
-		GstEvent *event = gst_event_new_stream_start(stream_id);
-		gst_event_set_group_id(event, state->group_id_);
-		gst_pad_push_event(srcpad, event);
+		gst_libcamera_pad_push_stream_start(srcpad, state->group_id_);
 
 		/* Collect the streams roles for the next iteration. */
 		roles.push_back(gst_libcamera_pad_get_role(srcpad));