[libcamera-devel] [PATCH v1] test: gstreamer: Simplify single stream test using functions from GstUtils

[paul.elder at ideasonboard.com]

Hi Vedant,

On Wed, Sep 15, 2021 at 08:39:07PM +0530, Vedant Paranjape wrote:
> Simplify memory handling and complexity of the test by using
> gst_parse_bin_from_description_full [1]. It also fixed memory leak reported
> by Umang Jain, described in his fix [2].

"For further information, see this patch that wasn't merged"

True, the patch isn't big or difficult to review, but you did squash two
patches that were doing distinct things (though they did have the same
end goal).

Please split the patches and expand the commit messages (copying what
Umang had is fine; he said so in the original patch series).


Paul

> 
> [1] https://gstreamer.freedesktop.org/documentation/gstreamer/gstutils.html?gi-language=c#gst_parse_bin_from_description_full
> [2] https://patchwork.libcamera.org/patch/13845/
> 
> Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> ---
>  .../gstreamer_single_stream_test.cpp          | 29 +++++++++----------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
> index 7292f3280617..a20d79109885 100644
> --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> @@ -33,20 +33,15 @@ protected:
>  		if (status_ != TestPass)
>  			return status_;
>  
> -		g_autoptr(GstElement) convert0 = gst_element_factory_make("videoconvert", "convert0");
> -		g_autoptr(GstElement) sink0 = gst_element_factory_make("fakesink", "sink0");
> -		g_object_ref_sink(convert0);
> -		g_object_ref_sink(sink0);
> -
> -		if (!convert0 || !sink0) {
> -			g_printerr("Not all elements could be created. %p.%p\n",
> -				   convert0, sink0);
> +		const gchar* streamDescription = "queue ! videoconvert ! 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("Not all bins could be created. %p\n", stream0_);
>  			return TestFail;
>  		}
> -
> -		convert0_ = reinterpret_cast<GstElement *>(g_steal_pointer(&convert0));
> -		sink0_ = reinterpret_cast<GstElement *>(g_steal_pointer(&sink0));
> +		g_object_ref_sink(stream0_);
>  
>  		if (createPipeline() != TestPass)
>  			return TestFail;
> @@ -57,8 +52,8 @@ protected:
>  	int run() override
>  	{
>  		/* Build the pipeline */
> -		gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, convert0_, sink0_, NULL);
> -		if (gst_element_link_many(libcameraSrc_, convert0_, sink0_, NULL) != TRUE) {
> +		gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, stream0_, NULL);
> +		if (gst_element_link(libcameraSrc_, stream0_) != TRUE) {
>  			g_printerr("Elements could not be linked.\n");
>  			return TestFail;
>  		}
> @@ -72,9 +67,13 @@ protected:
>  		return TestPass;
>  	}
>  
> +	void cleanup() override
> +	{
> +		g_clear_object(&stream0_);
> +	}
> +
>  private:
> -	GstElement *convert0_;
> -	GstElement *sink0_;
> +	GstElement *stream0_;
>  };
>  
>  TEST_REGISTER(GstreamerSingleStreamTest)
> -- 
> 2.25.1
>