[PATCH v2 2/3] test: gstreamer: Simplify single stream test
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon May 13 13:56:19 CEST 2024
On Thu, May 09, 2024 at 04:57:29PM +0100, Kieran Bingham wrote:
> Quoting Nicolas Dufresne (2024-05-09 16:12:18)
> > Hi,
> >
> > Le jeudi 09 mai 2024 à 15:50 +0100, Kieran Bingham a écrit :
> > > Quoting Nicolas Dufresne (2024-03-05 15:30:57)
> > > > From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > > >
> > >
> > > Why? Is it just better?
> >
> > I commit and sign with my Collabora address for all the work I do on my employer
> > time. But I'm using my personal address for mailing lists. I did no action here,
> > "git send-email" noticed and added the from. If you use "git am". it should
> > transparently reflect my choice here.
>
> Sorry - I meant - the commit message was empty. I don't mind (or care)
> which the From is from ;-)
>
> I meant "Simplify single stream test" - Why - is it better simplified?
> The empty commit message didn't tell us anything!
>
> > > I'll try to interpret as I read the patch. Please correct me whereever
> > > I'm wrong.
> > >
> > > """
> > > The single stream test for the gstreamer component has a simple pipeline
> > > construction using only a fakesink.
> > >
> > > The impelementation currently supports connecting to a more complex
s/impelementation/implementation/
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > stream construction defined by the streamDescription, but this is over
> > > engineered for the simple need to start a stream to capture and discard
> > > the frames.
> > >
> > > Convert the use of gst_parse_bin_from_description_full() which uses only
> > > a single element 'fakesink' to construct the fakesink directly and link
> > > it to the libcamerasrc.
> > > """
> >
> > That is very good, I should have wrote something, but if the target was the
> > GStreamer project, it would have been considered overkill due to the trivality
> > of the changes (from a GStreamer dev point of view of course). I'll keep in mind
> > for next time.
>
> The difficulty for me is I'm not a gstreamer dev, so I find the
> gstreamer element difficult to read. An introduction is very helpful to
> go into the patch with something in mind.
>
> If you're happy with the commit message, it can be added at applying or
> in a v2 - I don't know if a v2 is needed yet.
I'll merge this patch already, independently from the rest of the
series.
> > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > >
> > > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > >
> > > > ---
> > > > .../gstreamer_single_stream_test.cpp | 27 +++++++------------
> > > > 1 file changed, 9 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
> > > > index 301e4a93..815c6609 100644
> > > > --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> > > > +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> > > > @@ -29,30 +29,21 @@ protected:
> > > > if (status_ != TestPass)
> > > > return status_;
> > > >
> > > > - const gchar *streamDescription = "fakesink";
> > > > - g_autoptr(GError) error0 = NULL;
> > > > - stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,
> > > > - NULL,
> > > > - GST_PARSE_FLAG_FATAL_ERRORS,
> > > > - &error0);
> > > > -
> > > > - if (!stream0_) {
> > > > - g_printerr("Bin could not be created (%s)\n", error0->message);
> > > > + fakesink_ = gst_element_factory_make("fakesink", nullptr);
> > > > + if (!fakesink_) {
> > > > + g_printerr("Your installation is missing 'fakesink'\n");
> > > > return TestFail;
> > > > }
> > > > - g_object_ref_sink(stream0_);
> > > > -
> > > > - if (createPipeline() != TestPass)
> > > > - return TestFail;
> > > > + g_object_ref_sink(fakesink_);
> > > >
> > > > - return TestPass;
> > > > + return createPipeline();
> > > > }
> > > >
> > > > int run() override
> > > > {
> > > > /* Build the pipeline */
> > > > - gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, stream0_, NULL);
> > > > - if (gst_element_link(libcameraSrc_, stream0_) != TRUE) {
> > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, fakesink_, nullptr);
> > > > + if (!gst_element_link(libcameraSrc_, fakesink_)) {
> > > > g_printerr("Elements could not be linked.\n");
> > > > return TestFail;
> > > > }
> > > > @@ -68,11 +59,11 @@ protected:
> > > >
> > > > void cleanup() override
> > > > {
> > > > - g_clear_object(&stream0_);
> > > > + g_clear_object(&fakesink_);
> > > > }
> > > >
> > > > private:
> > > > - GstElement *stream0_;
> > > > + GstElement *fakesink_;
> > > > };
> > > >
> > > > TEST_REGISTER(GstreamerSingleStreamTest)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list