[{"id":19538,"web_url":"https://patchwork.libcamera.org/comment/19538/","msgid":"<20210908084007.GA4080653@pyrite.rasen.tech>","date":"2021-09-08T08:40:07","subject":"Re: [libcamera-devel] [PATCH v5] test: gstreamer: Factor out code\n\tinto a base class","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Vedant,\n\nOn Wed, Sep 08, 2021 at 01:24:42PM +0530, Vedant Paranjape wrote:\n> Lot of code used in single stream test is boiler plate and common across\n\ns/Lot/A lot/\n\n> every gstreamer test. Factored out this code into a base class called\n\ns/Factored/Factor/\n\n> GstreamerTest.\n> \n> Also updated the gstreamer_single_stream_test to use the GstreamerTest\n\ns/updated/update/\n\n> base class\n\ns/class/class./\n\n> \n> Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> ---\n>  .../gstreamer_single_stream_test.cpp          | 145 +++-------------\n>  test/gstreamer/gstreamer_test.cpp             | 156 ++++++++++++++++++\n>  test/gstreamer/gstreamer_test.h               |  39 +++++\n>  test/gstreamer/meson.build                    |   2 +-\n>  4 files changed, 220 insertions(+), 122 deletions(-)\n>  create mode 100644 test/gstreamer/gstreamer_test.cpp\n>  create mode 100644 test/gstreamer/gstreamer_test.h\n> \n> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp\n> index 4c8d4804..7292f328 100644\n> --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> @@ -14,104 +14,48 @@\n>  \n>  #include <gst/gst.h>\n>  \n> +#include \"gstreamer_test.h\"\n>  #include \"test.h\"\n>  \n>  using namespace std;\n>  \n> -extern \"C\" {\n> -const char *__asan_default_options()\n> -{\n> -\t/*\n> -\t * Disable leak detection due to a known global variable initialization\n> -\t * leak in glib's g_quark_init(). This should ideally be handled by\n> -\t * using a suppression file instead of disabling leak detection.\n> -\t */\n> -\treturn \"detect_leaks=false\";\n> -}\n> -}\n> -\n> -class GstreamerSingleStreamTest : public Test\n> +class GstreamerSingleStreamTest : public GstreamerTest, public Test\n>  {\n> +public:\n> +\tGstreamerSingleStreamTest()\n> +\t\t: GstreamerTest()\n> +\t{\n> +\t}\n> +\n>  protected:\n>  \tint init() override\n>  \t{\n> -\t\t/*\n> -\t\t * GStreamer by default spawns a process to run the\n> -\t\t * gst-plugin-scanner helper. If libcamera is compiled with ASan\n> -\t\t * enabled, and as GStreamer is most likely not, this causes the\n> -\t\t * ASan link order check to fail when gst-plugin-scanner\n> -\t\t * dlopen()s the plugin as many libraries will have already been\n> -\t\t * loaded by then. Fix this issue by disabling spawning of a\n> -\t\t * child helper process when scanning the build directory for\n> -\t\t * plugins.\n> -\t\t */\n> -\t\tgst_registry_fork_set_enabled(false);\n> -\n> -\t\t/* Initialize GStreamer */\n> -\t\tg_autoptr(GError) errInit = NULL;\n> -\t\tif (!gst_init_check(nullptr, nullptr, &errInit)) {\n> -\t\t\tg_printerr(\"Could not initialize GStreamer: %s\\n\",\n> -\t\t\t\t   errInit ? errInit->message : \"unknown error\");\n> +\t\tif (status_ != TestPass)\n> +\t\t\treturn status_;\n>  \n> -\t\t\treturn TestFail;\n> -\t\t}\n> +\t\tg_autoptr(GstElement) convert0 = gst_element_factory_make(\"videoconvert\", \"convert0\");\n> +\t\tg_autoptr(GstElement) sink0 = gst_element_factory_make(\"fakesink\", \"sink0\");\n> +\t\tg_object_ref_sink(convert0);\n> +\t\tg_object_ref_sink(sink0);\n>  \n> -\t\t/*\n> -\t\t * Remove the system libcamera plugin, if any, and add the\n> -\t\t * plugin from the build directory.\n> -\t\t */\n> -\t\tGstRegistry *registry = gst_registry_get();\n> -\t\tGstPlugin *plugin = gst_registry_lookup(registry, \"libgstlibcamera.so\");\n> -\t\tif (plugin) {\n> -\t\t\tgst_registry_remove_plugin(registry, plugin);\n> -\t\t\tgst_object_unref(plugin);\n> -\t\t}\n> +\t\tif (!convert0 || !sink0) {\n> +\t\t\tg_printerr(\"Not all elements could be created. %p.%p\\n\",\n> +\t\t\t\t   convert0, sink0);\n>  \n> -\t\tstd::string path = libcamera::utils::libcameraBuildPath()\n> -\t\t\t\t + \"src/gstreamer\";\n> -\t\tif (!gst_registry_scan_path(registry, path.c_str())) {\n> -\t\t\tg_printerr(\"Failed to add plugin to registry\\n\");\n> -\t\t\tgst_deinit();\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\t/* Create the elements */\n> -\t\tlibcameraSrc_ = gst_element_factory_make(\"libcamerasrc\", \"libcamera\");\n> -\t\tconvert0_ = gst_element_factory_make(\"videoconvert\", \"convert0\");\n> -\t\tsink0_ = gst_element_factory_make(\"fakesink\", \"sink0\");\n> -\n> -\t\t/* Create the empty pipeline_ */\n> -\t\tpipeline_ = gst_pipeline_new(\"test-pipeline\");\n> -\n> -\t\tif (!pipeline_ || !convert0_ || !sink0_ || !libcameraSrc_) {\n> -\t\t\tg_printerr(\"Not all elements could be created. %p.%p.%p.%p\\n\",\n> -\t\t\t\t   pipeline_, convert0_, sink0_, libcameraSrc_);\n> -\t\t\tif (pipeline_)\n> -\t\t\t\tgst_object_unref(pipeline_);\n> -\t\t\tif (convert0_)\n> -\t\t\t\tgst_object_unref(convert0_);\n> -\t\t\tif (sink0_)\n> -\t\t\t\tgst_object_unref(sink0_);\n> -\t\t\tif (libcameraSrc_)\n> -\t\t\t\tgst_object_unref(libcameraSrc_);\n> -\t\t\tgst_deinit();\n> +\t\tconvert0_ = reinterpret_cast<GstElement *>(g_steal_pointer(&convert0));\n> +\t\tsink0_ = reinterpret_cast<GstElement *>(g_steal_pointer(&sink0));\n>  \n> +\t\tif (createPipeline() != TestPass)\n>  \t\t\treturn TestFail;\n> -\t\t}\n>  \n>  \t\treturn TestPass;\n>  \t}\n>  \n> -\tvoid cleanup() override\n> -\t{\n> -\t\tgst_object_unref(pipeline_);\n> -\t\tgst_deinit();\n> -\t}\n> -\n>  \tint run() override\n>  \t{\n> -\t\tGstStateChangeReturn ret;\n> -\n>  \t\t/* Build the pipeline */\n>  \t\tgst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, convert0_, sink0_, NULL);\n>  \t\tif (gst_element_link_many(libcameraSrc_, convert0_, sink0_, NULL) != TRUE) {\n> @@ -119,57 +63,16 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> -\t\t/* Start playing */\n> -\t\tret = gst_element_set_state(pipeline_, GST_STATE_PLAYING);\n> -\t\tif (ret == GST_STATE_CHANGE_FAILURE) {\n> -\t\t\tg_printerr(\"Unable to set the pipeline to the playing state.\\n\");\n> +\t\tif (startPipeline() != TestPass)\n>  \t\t\treturn TestFail;\n> -\t\t}\n>  \n> -\t\t/* Wait until error or EOS or timeout after 2 seconds */\n> -\t\tconstexpr GstMessageType msgType =\n> -\t\t\tstatic_cast<GstMessageType>(GST_MESSAGE_ERROR | GST_MESSAGE_EOS);\n> -\t\tconstexpr GstClockTime timeout = 2 * GST_SECOND;\n> -\n> -\t\tg_autoptr(GstBus) bus = gst_element_get_bus(pipeline_);\n> -\t\tg_autoptr(GstMessage) msg = gst_bus_timed_pop_filtered(bus, timeout, msgType);\n> -\n> -\t\tgst_element_set_state(pipeline_, GST_STATE_NULL);\n> -\n> -\t\t/* Parse error message */\n> -\t\tif (msg == NULL)\n> -\t\t\treturn TestPass;\n> -\n> -\t\tswitch (GST_MESSAGE_TYPE(msg)) {\n> -\t\tcase GST_MESSAGE_ERROR:\n> -\t\t\tgstreamer_print_error(msg);\n> -\t\t\tbreak;\n> -\t\tcase GST_MESSAGE_EOS:\n> -\t\t\tg_print(\"End-Of-Stream reached.\\n\");\n> -\t\t\tbreak;\n> -\t\tdefault:\n> -\t\t\tg_printerr(\"Unexpected message received.\\n\");\n> -\t\t\tbreak;\n> -\t\t}\n> +\t\tif (processEvent() != TestPass)\n> +\t\t\treturn TestFail;\n>  \n> -\t\treturn TestFail;\n> +\t\treturn TestPass;\n>  \t}\n>  \n>  private:\n> -\tvoid gstreamer_print_error(GstMessage *msg)\n> -\t{\n> -\t\tg_autoptr(GError) err = NULL;\n> -\t\tg_autofree gchar *debug_info = NULL;\n> -\n> -\t\tgst_message_parse_error(msg, &err, &debug_info);\n> -\t\tg_printerr(\"Error received from element %s: %s\\n\",\n> -\t\t\t   GST_OBJECT_NAME(msg->src), err->message);\n> -\t\tg_printerr(\"Debugging information: %s\\n\",\n> -\t\t\t   debug_info ? debug_info : \"none\");\n> -\t}\n> -\n> -\tGstElement *pipeline_;\n> -\tGstElement *libcameraSrc_;\n>  \tGstElement *convert0_;\n>  \tGstElement *sink0_;\n>  };\n> diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp\n> new file mode 100644\n> index 00000000..1baecee1\n> --- /dev/null\n> +++ b/test/gstreamer/gstreamer_test.cpp\n> @@ -0,0 +1,156 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2021, Vedant Paranjape\n> + *\n> + * libcamera Gstreamer element API tests\n> + */\n> +\n> +#include \"gstreamer_test.h\"\n> +\n> +#include \"test.h\"\n> +\n> +using namespace std;\n> +\n> +extern \"C\" {\n> +const char *__asan_default_options()\n> +{\n> +\t/*\n> +\t * Disable leak detection due to a known global variable initialization\n> +\t * leak in glib's g_quark_init(). This should ideally be handled by\n> +\t * using a suppression file instead of disabling leak detection.\n> +\t */\n> +\treturn \"detect_leaks=false\";\n> +}\n> +}\n> +\n> +GstreamerTest::GstreamerTest()\n> +{\n> +\t/*\n> +\t* GStreamer by default spawns a process to run the\n> +\t* gst-plugin-scanner helper. If libcamera is compiled with ASan\n> +\t* enabled, and as GStreamer is most likely not, this causes the\n> +\t* ASan link order check to fail when gst-plugin-scanner\n> +\t* dlopen()s the plugin as many libraries will have already been\n> +\t* loaded by then. Fix this issue by disabling spawning of a\n> +\t* child helper process when scanning the build directory for\n> +\t* plugins.\n> +\t*/\n> +\tgst_registry_fork_set_enabled(false);\n> +\n> +\t/* Initialize GStreamer */\n> +\tg_autoptr(GError) errInit = NULL;\n> +\tif (!gst_init_check(nullptr, nullptr, &errInit)) {\n> +\t\tg_printerr(\"Could not initialize GStreamer: %s\\n\",\n> +\t\t\t   errInit ? errInit->message : \"unknown error\");\n> +\n> +\t\tstatus_ = TestFail;\n> +\t\treturn;\n> +\t}\n> +\n> +\t/*\n> +\t* Remove the system libcamera plugin, if any, and add the\n> +\t* plugin from the build directory.\n> +\t*/\n> +\tGstRegistry *registry = gst_registry_get();\n> +\tg_autoptr(GstPlugin) plugin = gst_registry_lookup(registry, \"libgstlibcamera.so\");\n> +\tif (plugin)\n> +\t\tgst_registry_remove_plugin(registry, plugin);\n> +\n> +\tstd::string path = libcamera::utils::libcameraBuildPath() + \"src/gstreamer\";\n> +\tif (!gst_registry_scan_path(registry, path.c_str())) {\n> +\t\tg_printerr(\"Failed to add plugin to registry\\n\");\n> +\n> +\t\tstatus_ = TestFail;\n> +\t\treturn;\n> +\t}\n> +\n> +\tstatus_ = TestPass;\n> +}\n> +\n> +GstreamerTest::~GstreamerTest()\n> +{\n> +\tif (libcameraSrc_ &&\n> +\t    !gst_object_has_as_ancestor(GST_OBJECT(libcameraSrc_),\n> +\t\t\t\t\tGST_OBJECT(pipeline_)))\n> +\t\tgst_object_unref(libcameraSrc_);\n> +\tif (pipeline_)\n> +\t\tgst_object_unref(pipeline_);\n> +\n> +\tgst_deinit();\n> +}\n> +\n> +int GstreamerTest::createPipeline()\n> +{\n> +\tg_autoptr(GstElement) libcameraSrc = gst_element_factory_make(\"libcamerasrc\", \"libcamera\");\n> +\tpipeline_ = gst_pipeline_new(\"test-pipeline\");\n> +\tg_object_ref_sink(libcameraSrc);\n> +\n> +\tif (!libcameraSrc || !pipeline_) {\n> +\t\tg_printerr(\"Unable to create create pipeline %p.%p\\n\",\n> +\t\t\t   libcameraSrc, pipeline_);\n> +\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tlibcameraSrc_ = reinterpret_cast<GstElement *>(g_steal_pointer(&libcameraSrc));\n> +\n> +\treturn TestPass;\n> +}\n> +\n> +int GstreamerTest::startPipeline()\n> +{\n> +\tGstStateChangeReturn ret;\n> +\n> +\t/* Start playing */\n> +\tret = gst_element_set_state(pipeline_, GST_STATE_PLAYING);\n> +\tif (ret == GST_STATE_CHANGE_FAILURE) {\n> +\t\tg_printerr(\"Unable to set the pipeline to the playing state.\\n\");\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\treturn TestPass;\n> +}\n> +\n> +int GstreamerTest::processEvent()\n> +{\n> +\t/* Wait until error or EOS or timeout after 2 seconds */\n> +\tconstexpr GstMessageType msgType =\n> +\t\tstatic_cast<GstMessageType>(GST_MESSAGE_ERROR | GST_MESSAGE_EOS);\n> +\tconstexpr GstClockTime timeout = 2 * GST_SECOND;\n> +\n> +\tg_autoptr(GstBus) bus = gst_element_get_bus(pipeline_);\n> +\tg_autoptr(GstMessage) msg = gst_bus_timed_pop_filtered(bus, timeout, msgType);\n> +\n> +\tgst_element_set_state(pipeline_, GST_STATE_NULL);\n> +\n> +\t/* Parse error message */\n> +\tif (msg == NULL)\n> +\t\treturn TestPass;\n> +\n> +\tswitch (GST_MESSAGE_TYPE(msg)) {\n> +\tcase GST_MESSAGE_ERROR:\n> +\t\tprintError(msg);\n> +\t\tbreak;\n> +\tcase GST_MESSAGE_EOS:\n> +\t\tg_print(\"End-Of-Stream reached.\\n\");\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tg_printerr(\"Unexpected message received.\\n\");\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn TestFail;\n> +}\n> +\n> +void GstreamerTest::printError(GstMessage *msg)\n> +{\n> +\tg_autoptr(GError) err = NULL;\n> +\tg_autofree gchar *debug_info = NULL;\n> +\n> +\tgst_message_parse_error(msg, &err, &debug_info);\n> +\tg_printerr(\"Error received from element %s: %s\\n\",\n> +\t\t   GST_OBJECT_NAME(msg->src), err->message);\n> +\tg_printerr(\"Debugging information: %s\\n\",\n> +\t\t   debug_info ? debug_info : \"none\");\n> +}\n> +\n\nYou have an extra line here.\n\n\nI'll fix these when applying.\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h\n> new file mode 100644\n> index 00000000..9c50e288\n> --- /dev/null\n> +++ b/test/gstreamer/gstreamer_test.h\n> @@ -0,0 +1,39 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2021, Vedant Paranjape\n> + *\n> + * gstreamer_test.cpp - GStreamer test base class\n> + */\n> +\n> +#ifndef __LIBCAMERA_GSTREAMER_TEST_H__\n> +#define __LIBCAMERA_GSTREAMER_TEST_H__\n> +\n> +#include <iostream>\n> +#include <unistd.h>\n> +\n> +#include <libcamera/base/utils.h>\n> +\n> +#include \"libcamera/internal/source_paths.h\"\n> +\n> +#include <gst/gst.h>\n> +\n> +using namespace std;\n> +\n> +class GstreamerTest\n> +{\n> +public:\n> +\tGstreamerTest();\n> +\tvirtual ~GstreamerTest();\n> +\n> +protected:\n> +\tvirtual int createPipeline();\n> +\tint startPipeline();\n> +\tint processEvent();\n> +\tvoid printError(GstMessage *msg);\n> +\n> +\tGstElement *pipeline_;\n> +\tGstElement *libcameraSrc_;\n> +\tint status_;\n> +};\n> +\n> +#endif /* __LIBCAMERA_GSTREAMER_TEST_H__ */\n> diff --git a/test/gstreamer/meson.build b/test/gstreamer/meson.build\n> index b99aa0da..aca53b92 100644\n> --- a/test/gstreamer/meson.build\n> +++ b/test/gstreamer/meson.build\n> @@ -10,7 +10,7 @@ gstreamer_tests = [\n>  gstreamer_dep = dependency('gstreamer-1.0', required: true)\n>  \n>  foreach t : gstreamer_tests\n> -    exe = executable(t[0], t[1],\n> +    exe = executable(t[0], t[1], 'gstreamer_test.cpp',\n>                       dependencies : [libcamera_private, gstreamer_dep],\n>                       link_with : test_libraries,\n>                       include_directories : test_includes_internal)\n> -- \n> 2.25.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6B5DBBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  8 Sep 2021 08:40:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6DEE69170;\n\tWed,  8 Sep 2021 10:40:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6D7566024D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  8 Sep 2021 10:40:15 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8A771993;\n\tWed,  8 Sep 2021 10:40:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pjZ3AihV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631090415;\n\tbh=SDhB3iHn56/EuRXCmp8VgSuivCDBHa4+CUcAodlsRb8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pjZ3AihV4GMx3sUD2ULaYIyq+yHOtLuCSaZyw6j1fopl+oSN+p7XfB3g2FQ36Lg1N\n\tU6VUHqvDHpNFIgALwBRFwBV8xMbzM9LBORXzx9/MMtD2p6vJvHfOtXXN7hVgP6qLaI\n\teLEn4DbQ6xBQL0HSRelMFEDXzQMlUb5qYSsIh09Y=","Date":"Wed, 8 Sep 2021 17:40:07 +0900","From":"paul.elder@ideasonboard.com","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Message-ID":"<20210908084007.GA4080653@pyrite.rasen.tech>","References":"<20210908075442.128782-1-vedantparanjape160201@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210908075442.128782-1-vedantparanjape160201@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v5] test: gstreamer: Factor out code\n\tinto a base class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne <nicolas.dufresne@collabora.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]