<html><head></head><body><div>Le mardi 14 septembre 2021 à 23:16 +0530, Vedant Paranjape a écrit :</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="auto"><div>Hi Nicolas,</div><div dir="auto"><br><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Tue, 14 Sep, 2021, 23:08 Nicolas Dufresne, <<a href="mailto:nicolas@ndufresne.ca">nicolas@ndufresne.ca</a>> wrote:<br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>Le vendredi 10 septembre 2021 à 23:41 +0530, Vedant Paranjape a écrit :<br>> This patch adds a test to test if multi stream using libcamera's<br>> gstreamer element works.<br>> <br>> Signed-off-by: Vedant Paranjape <<a href="mailto:vedantparanjape160201@gmail.com" target="_blank" rel="noreferrer">vedantparanjape160201@gmail.com</a>><br>> ---<br>> .../gstreamer/gstreamer_multi_stream_test.cpp | 138 ++++++++++++++++++<br>> test/gstreamer/meson.build | 1 +<br>> 2 files changed, 139 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..e5c909c85da2<br>> --- /dev/null<br>> +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp<br>> @@ -0,0 +1,138 @@<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>> +#include <libcamera/libcamera.h><br>> +<br>> +#include "libcamera/internal/source_paths.h"<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 &i : cm.cameras()) {<br>> + if (i->streams().size() > 1) {<br>> + cameraName = i->id();<br>> + cameraFound = true;<br>> + cm.stop();<br>> + break;<br>> + }<br>> + }<br>> +<br>> + if (!cameraFound) {<br>> + cm.stop();<br>> + return TestSkip;<br>> + }<br>> +<br>> + g_autoptr(GstElement) convert0 = gst_element_factory_make("videoconvert", "convert0");<br>> + g_autoptr(GstElement) convert1 = gst_element_factory_make("videoconvert", "convert1");<br>> + g_autoptr(GstElement) sink0 = gst_element_factory_make("fakesink", "sink0");<br>> + g_autoptr(GstElement) sink1 = gst_element_factory_make("fakesink", "sink1");<br>> + g_autoptr(GstElement) queue0 = gst_element_factory_make("queue", "queue0");<br>> + g_autoptr(GstElement) queue1 = gst_element_factory_make("queue", "queue1");<br>> + g_object_ref_sink(convert0);<br>> + g_object_ref_sink(convert1);<br>> + g_object_ref_sink(sink0);<br>> + g_object_ref_sink(sink1);<br>> + g_object_ref_sink(queue0);<br>> + g_object_ref_sink(queue1);<br></div><div><br>ref_sink should be inline or next to their allocation.<br></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Okay, will change.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><br>> +<br>> + if (!convert0 || !convert1 || !sink0 || !sink1 || !queue0 || !queue1) {<br>> + g_printerr("Not all elements could be created. %p.%p.%p.%p.%p.%p\n",<br>> + convert0, convert1, sink0, sink1, queue0, queue1);<br>> +<br>> + return TestFail;<br>> + }<br>> +<br>> + convert0_ = reinterpret_cast<GstElement *>(g_steal_pointer(&convert0));<br>> + convert1_ = reinterpret_cast<GstElement *>(g_steal_pointer(&convert1));<br>> + sink0_ = reinterpret_cast<GstElement *>(g_steal_pointer(&sink0));<br>> + sink1_ = reinterpret_cast<GstElement *>(g_steal_pointer(&sink1));<br>> + queue0_ = reinterpret_cast<GstElement *>(g_steal_pointer(&queue0));<br>> + queue1_ = reinterpret_cast<GstElement *>(g_steal_pointer(&queue1));<br></div><div><br>What is the point of using autoptr here, you will clean them up in your<br>destructure, just store them directly in the class. In general, if you feel the<br>code is ugly, that's likely before you are doing something that is not needed.<br></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Okay, makes sense.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>> +<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_, queue0_, queue1_,<br>> + convert0_, convert1_, sink0_, sink1_, NULL);<br>> + if (gst_element_link_many(queue0_, convert0_, sink0_, NULL) != TRUE<br>> + || gst_element_link_many(queue1_, convert1_, sink1_, NULL) != TRUE) {<br>> + g_printerr("Elements could not be linked.\n");<br>> + return TestFail;<br>> + }<br></div><div><br>I think we'd gain in readability of using gst_parse_launch_full(). You can even<br>link requested pad with that. It's also very ackward here, since you have a<br>run() function which cannot be run twice.<br></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I agree with this, I'll take a look at the function.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>> +<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>> + GstPad *queue0_sink_pad = gst_element_get_static_pad(queue0_, "sink");<br>> + GstPad *queue1_sink_pad = gst_element_get_static_pad(queue1_, "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>> + if (queue0_sink_pad)<br>> + gst_object_unref(queue0_sink_pad);<br>> + if (queue1_sink_pad)<br>> + gst_object_unref(queue1_sink_pad);<br>> + g_printerr("Pads could not be linked.\n");<br>> + return TestFail;<br>> + }<br>> + gst_object_unref(queue0_sink_pad);<br>> + gst_object_unref(queue1_sink_pad);<br>> +<br>> + if (startPipeline() != TestPass)<br>> + return TestFail;<br>> +<br>> + if (processEvent() != TestPass)<br>> + return TestFail;<br>> +<br>> + return TestPass;<br>> + }<br>> +<br>> +private:<br>> + std::string cameraName;<br>> + GstElement *convert0_;<br>> + GstElement *convert1_;<br>> + GstElement *sink0_;<br>> + GstElement *sink1_;<br>> + GstElement *queue0_;<br>> + GstElement *queue1_;<br></div><div><br>No destructor ? All leaked ?<br></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">How ? I have added all to pipeline, it will take care of their lifetime.</div></div></blockquote><div><br></div><div>Well, if you remove the floating ref, ownership is not transferred to the pipeline and you have to unref when you are done. That is the core principal of floating references.</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="auto"><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>> +};<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>> gstreamer_dep = dependency('gstreamer-1.0', required: true)<br>> <br></div><div><br></div><div><br></div></blockquote></div></div></div></blockquote><div><br></div><div><span></span></div></body></html>