<div dir="auto"><div>Hi Paul,<div dir="auto">Can you also tell me the expected indentation ?</div><div dir="auto"><br></div><div dir="auto">checkstyle suggestions didn't adhere with the 80 char width thing</div><div dir="auto"><br></div><div dir="auto"><br></div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 24 Sep, 2021, 11:39 , <<a href="mailto:paul.elder@ideasonboard.com">paul.elder@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Vedant,<br>
<br>
On Thu, Sep 23, 2021 at 08:26:37PM +0530, Vedant Paranjape wrote:<br>
> This patch adds a test to test if multi stream using libcamera's<br>
> gstreamer element works.<br>
> <br>
> Test will run only on devices that support multistream output, i.e.,<br>
<br>
I think s/output/capture/ might be better.<br>
<br>
Also s/i.e./e.g/<br>
<br>
> devices that use IPU3 and raspberrypi pipeline. This was tested on<br>
> a Raspberry Pi 4B+.<br>
> <br>
> Signed-off-by: Vedant Paranjape <<a href="mailto:vedantparanjape160201@gmail.com" target="_blank" rel="noreferrer">vedantparanjape160201@gmail.com</a>><br>
> Reviewed-by: Nicolas Dufresne <<a href="mailto:nicolas.dufresne@collabora.com" target="_blank" rel="noreferrer">nicolas.dufresne@collabora.com</a>><br>
> ---<br>
> .../gstreamer/gstreamer_multi_stream_test.cpp | 128 ++++++++++++++++++<br>
> test/gstreamer/meson.build | 1 +<br>
> 2 files changed, 129 insertions(+)<br>
> create mode 100644 test/gstreamer/gstreamer_multi_stream_test.cpp<br>
> <br>
> diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/gstreamer/gstreamer_multi_stream_test.cpp<br>
> new file mode 100644<br>
> index 000000000000..6f63f1e748e2<br>
> --- /dev/null<br>
> +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp<br>
> @@ -0,0 +1,128 @@<br>
> +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> +/*<br>
> + * Copyright (C) 2021, Vedant Paranjape<br>
> + *<br>
> + * gstreamer_multi_stream_test.cpp - GStreamer multi stream capture test<br>
> + */<br>
> +<br>
> +#include <iostream><br>
> +#include <unistd.h><br>
> +<br>
> +#include <libcamera/base/utils.h><br>
<br>
You don't use anything from this.<br>
<br>
> +<br>
> +#include <libcamera/libcamera.h><br>
> +<br>
> +#include "libcamera/internal/source_paths.h"<br>
<br>
Nor this.<br>
<br>
(The single stream tests have these too)<br>
<br>
> +<br>
> +#include <gst/gst.h><br>
> +<br>
> +#include "gstreamer_test.h"<br>
> +#include "test.h"<br>
> +<br>
> +using namespace std;<br>
> +<br>
> +class GstreamerMultiStreamTest : public GstreamerTest, public Test<br>
> +{<br>
> +public:<br>
> + GstreamerMultiStreamTest()<br>
> + : GstreamerTest()<br>
> + {<br>
> + }<br>
> +<br>
> +protected:<br>
> + int init() override<br>
> + {<br>
> + if (status_ != TestPass)<br>
> + return status_;<br>
> +<br>
> + /* Check if platform support multistream output */<br>
> + libcamera::CameraManager cm;<br>
> + cm.start();<br>
> + bool cameraFound = false;<br>
> + for (auto &camera : cm.cameras()) {<br>
> + if (camera->streams().size() > 1) {<br>
> + cameraName = camera->id();<br>
> + cameraFound = true;<br>
> + cm.stop();<br>
> + break;<br>
> + }<br>
> + }<br>
> +<br>
> + if (!cameraFound) {<br>
> + cm.stop();<br>
> + return TestSkip;<br>
> + }<br>
> +<br>
> + const gchar *streamDescription = "queue ! fakesink";<br>
> + g_autoptr(GError) error = NULL;<br>
> +<br>
> + stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,<br>
> + NULL,<br>
> + GST_PARSE_FLAG_FATAL_ERRORS,<br>
> + &error);<br>
> + if (!stream0_) {<br>
> + g_printerr("Stream0 could not be created (%s)\n", error->message);<br>
> + return TestFail;<br>
> + }<br>
> + g_object_ref_sink(stream0_);<br>
> +<br>
> + stream1_ = gst_parse_bin_from_description_full(streamDescription, TRUE,<br>
> + NULL,<br>
> + GST_PARSE_FLAG_FATAL_ERRORS,<br>
> + &error);<br>
> + if (!stream1_) {<br>
> + g_printerr("Stream1 could not be created (%s)\n", error->message);<br>
> + return TestFail;<br>
> + }<br>
> + g_object_ref_sink(stream1_);<br>
> +<br>
> + if (createPipeline() != TestPass)<br>
> + return TestFail;<br>
> +<br>
> + return TestPass;<br>
> + }<br>
> +<br>
> + int run() override<br>
> + {<br>
> + g_object_set(libcameraSrc_, "camera-name", cameraName.c_str(), NULL);<br>
> +<br>
> + /* Build the pipeline */<br>
> + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,<br>
> + stream0_, stream1_, NULL);<br>
<br>
Indentation.<br>
<br>
> +<br>
> + g_autoptr(GstPad) src_pad = gst_element_get_static_pad(libcameraSrc_, "src");<br>
> + g_autoptr(GstPad) request_pad = gst_element_get_request_pad(libcameraSrc_, "src_%u");<br>
> +<br>
> + {<br>
> + g_autoptr(GstPad) queue0_sink_pad = gst_element_get_static_pad(stream0_, "sink");<br>
> + g_autoptr(GstPad) queue1_sink_pad = gst_element_get_static_pad(stream1_, "sink");<br>
> +<br>
> + if (gst_pad_link(src_pad, queue0_sink_pad) != GST_PAD_LINK_OK<br>
> + || gst_pad_link(request_pad, queue1_sink_pad) != GST_PAD_LINK_OK) {<br>
<br>
Indentation.<br>
<br>
> + g_printerr("Pads could not be linked.\n");<br>
> + return TestFail;<br>
> + }<br>
> + }<br>
> +<br>
> + if (startPipeline() != TestPass)<br>
> + return TestFail;<br>
> +<br>
> + if (processEvent() != TestPass)<br>
> + return TestFail;<br>
> +<br>
> + return TestPass;<br>
> + }<br>
> +<br>
> + void cleanup() override<br>
> + {<br>
> + g_clear_object(&stream0_);<br>
> + g_clear_object(&stream1_);<br>
> + }<br>
> +<br>
> +private:<br>
> + std::string cameraName;<br>
<br>
s/cameraName/cameraName_/<br>
<br>
> + GstElement *stream0_;<br>
> + GstElement *stream1_;<br>
> +};<br>
> +<br>
> +TEST_REGISTER(GstreamerMultiStreamTest)<br>
> diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build<br>
> index aca53b920365..13652e87d05c 100644<br>
> --- a/test/gstreamer/meson.build<br>
> +++ b/test/gstreamer/meson.build<br>
> @@ -6,6 +6,7 @@ endif<br>
> <br>
> gstreamer_tests = [<br>
> ['single_stream_test', 'gstreamer_single_stream_test.cpp'],<br>
> + ['multi_stream_test', 'gstreamer_multi_stream_test.cpp'],<br>
<br>
It would be nice if the second elements lined up.<br>
<br>
<br>
Otherwise, looks good.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Can I add a R-b then?</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Paul<br>
<br>
> ]<br>
> gstreamer_dep = dependency('gstreamer-1.0', required: true)<br>
> <br>
> -- <br>
> 2.25.1<br>
> <br>
</blockquote></div></div></div>