[libcamera-devel,6/6] test: gstreamer: Remove videoconvert element from pipeline
diff mbox series

Message ID 20231206210524.20974-7-laurent.pinchart@ideasonboard.com
State Accepted
Commit 17d8b89deb1feb35ceba2f1310f4eea35fcb1d73
Headers show
Series
  • test: Miscellaneous fixes and improvements for unit tests
Related show

Commit Message

Laurent Pinchart Dec. 6, 2023, 9:05 p.m. UTC 
The GStreamer single stream test uses the following pipeline:

	libcamerasrc ! videoconvert ! fakesink

The videoconvert element isn't useful as the data is thrown away by the
fakesink anyway. We can shorten the pipeline to

	libcamerasrc ! fakesink

to save CPU time and to avoid depending on the gstreamer1.0-plugins-base
package to run the unit tests.

The test could be further simplified by replacing
gst_parse_bin_from_description_full() with gst_element_factory_make(),
now that we only add one element to the bin. The extra cost incurred by
the bin only impacts initialization time, and using a bin will make it
easier to add other elements in the future if needed. Keep the bin, and
only drop the videoconvert element.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 test/gstreamer/gstreamer_single_stream_test.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nicolas Dufresne Dec. 7, 2023, 3:45 p.m. UTC | #1 
Le mercredi 06 décembre 2023 à 23:05 +0200, Laurent Pinchart via libcamera-devel
a écrit :
> The GStreamer single stream test uses the following pipeline:
> 
> 	libcamerasrc ! videoconvert ! fakesink
> 
> The videoconvert element isn't useful as the data is thrown away by the
> fakesink anyway. We can shorten the pipeline to
> 
> 	libcamerasrc ! fakesink
> 
> to save CPU time and to avoid depending on the gstreamer1.0-plugins-base
> package to run the unit tests.
> 
> The test could be further simplified by replacing
> gst_parse_bin_from_description_full() with gst_element_factory_make(),
> now that we only add one element to the bin. The extra cost incurred by
> the bin only impacts initialization time, and using a bin will make it
> easier to add other elements in the future if needed. Keep the bin, and
> only drop the videoconvert element.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  test/gstreamer/gstreamer_single_stream_test.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
> index a0dd12cfea84..301e4a935a37 100644
> --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> @@ -29,7 +29,7 @@ protected:
>  		if (status_ != TestPass)
>  			return status_;
>  
> -		const gchar *streamDescription = "videoconvert ! fakesink";
> +		const gchar *streamDescription = "fakesink";
>  		g_autoptr(GError) error0 = NULL;
>  		stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,
>  						NULL,
Kieran Bingham Dec. 7, 2023, 4:30 p.m. UTC | #2 
Quoting Laurent Pinchart via libcamera-devel (2023-12-06 21:05:24)
> The GStreamer single stream test uses the following pipeline:
> 
>         libcamerasrc ! videoconvert ! fakesink
> 
> The videoconvert element isn't useful as the data is thrown away by the
> fakesink anyway. We can shorten the pipeline to
> 
>         libcamerasrc ! fakesink
> 
> to save CPU time and to avoid depending on the gstreamer1.0-plugins-base
> package to run the unit tests.
> 
> The test could be further simplified by replacing
> gst_parse_bin_from_description_full() with gst_element_factory_make(),
> now that we only add one element to the bin. The extra cost incurred by
> the bin only impacts initialization time, and using a bin will make it
> easier to add other elements in the future if needed. Keep the bin, and
> only drop the videoconvert element.

Yes, that sounds fine. It also helps show how pipeline can be
constructed, so it is also helpful keeping it this way.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  test/gstreamer/gstreamer_single_stream_test.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
> index a0dd12cfea84..301e4a935a37 100644
> --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> @@ -29,7 +29,7 @@ protected:
>                 if (status_ != TestPass)
>                         return status_;
>  
> -               const gchar *streamDescription = "videoconvert ! fakesink";
> +               const gchar *streamDescription = "fakesink";
>                 g_autoptr(GError) error0 = NULL;
>                 stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,
>                                                 NULL,
> -- 
> Regards,
> 
> Laurent Pinchart
>
Umang Jain Dec. 8, 2023, 7:13 a.m. UTC | #3 
Hi Laurent,

On 12/7/23 2:35 AM, Laurent Pinchart via libcamera-devel wrote:
> The GStreamer single stream test uses the following pipeline:
>
> 	libcamerasrc ! videoconvert ! fakesink
>
> The videoconvert element isn't useful as the data is thrown away by the
> fakesink anyway. We can shorten the pipeline to
>
> 	libcamerasrc ! fakesink
>
> to save CPU time and to avoid depending on the gstreamer1.0-plugins-base
> package to run the unit tests.
>
> The test could be further simplified by replacing
> gst_parse_bin_from_description_full() with gst_element_factory_make(),
> now that we only add one element to the bin. The extra cost incurred by
> the bin only impacts initialization time, and using a bin will make it
> easier to add other elements in the future if needed. Keep the bin, and
> only drop the videoconvert element.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>   test/gstreamer/gstreamer_single_stream_test.cpp | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
> index a0dd12cfea84..301e4a935a37 100644
> --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> @@ -29,7 +29,7 @@ protected:
>   		if (status_ != TestPass)
>   			return status_;
>   
> -		const gchar *streamDescription = "videoconvert ! fakesink";
> +		const gchar *streamDescription = "fakesink";
>   		g_autoptr(GError) error0 = NULL;
>   		stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,
>   						NULL,

Patch
diff mbox series 

diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
index a0dd12cfea84..301e4a935a37 100644
--- a/test/gstreamer/gstreamer_single_stream_test.cpp
+++ b/test/gstreamer/gstreamer_single_stream_test.cpp
@@ -29,7 +29,7 @@  protected:
 		if (status_ != TestPass)
 			return status_;
 
-		const gchar *streamDescription = "videoconvert ! fakesink";
+		const gchar *streamDescription = "fakesink";
 		g_autoptr(GError) error0 = NULL;
 		stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,
 						NULL,