[PATCH v2 2/3] test: gstreamer: Simplify single stream test

[Kieran Bingham]

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
> > 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.


> 
> > 
> > > 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)
> > > -- 
> > > 2.43.2
> > > 
>