[{"id":19185,"web_url":"https://patchwork.libcamera.org/comment/19185/","msgid":"<969940441892639c57d95f77d96d7a4a5d29dcf3.camel@ndufresne.ca>","date":"2021-08-30T20:36:10","subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Factor out code\n\tinto a base class","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Thanks for the patch,\n\nsee inline comment for possible improvements.\n\nLe lundi 30 août 2021 à 02:36 +0530, Vedant Paranjape a écrit :\n> Lot of code used in single stream test is boiler plate and common across\n> every gstreamer test. Factored out this code into a base class called\n> GstreamerTest.\n> \n> Also updated the gstreamer_single_stream_test to use the GstreamerTest\n> base class\n> \n> Signed-off-by: Vedant Paranjape \n> ---\n> .../gstreamer_single_stream_test.cpp | 138 +++------------\n> test/gstreamer/gstreamer_test.cpp | 157 ++++++++++++++++++\n> test/gstreamer/gstreamer_test.h | 38 +++++\n> test/gstreamer/meson.build | 2 +-\n> 4 files changed, 216 insertions(+), 119 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..021ac269 100644\n> --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> @@ -14,86 +14,35 @@\n> \n> #include \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> -\n> -\t\t\treturn TestFail;\n> -\t\t}\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> -\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> +\t\tif (status_ != TestPass)\n> +\t\t\treturn status_;\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\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> \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> \n> \t\t\treturn TestFail;\n> @@ -102,16 +51,8 @@ protected:\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 +60,18 @@ 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\t\treturn TestFail;\n> -\t\t}\n> +\t\tgstreamer_start_pipeline();\n> +\t\tif (status_ != TestPass)\n> +\t\t\treturn status_;\n> \n> -\t\t/* Wait until error or EOS or timeout after 2 seconds */\n> -\t\tconstexpr GstMessageType msgType =\n> -\t\t\tstatic_cast(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\tgstreamer_wait_for_event();\n> +\t\tif (status_ != TestPass)\n> +\t\t\treturn status_;\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..34608a02\n> --- /dev/null\n> +++ b/test/gstreamer/gstreamer_test.cpp\n> @@ -0,0 +1,157 @@\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\t* Remove the system libcamera plugin, if any, and add the\n> +\t\t* plugin from the build directory.\n> +\t\t*/\n\nIndent looks wrong, did you enable the commit hooks ?\n\n> +\tGstRegistry *registry = gst_registry_get();\n> +\tGstPlugin *plugin = gst_registry_lookup(registry, \"libgstlibcamera.so\");\n> +\tif (plugin) {\n> +\t\tgst_registry_remove_plugin(registry, plugin);\n> +\t\tgst_object_unref(plugin);\n\nnit: Perhaps use a g_autoptr() instead ?\n\n> +\t}\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> +\t\tgst_deinit();\n\nMove this in the destructor ? Note that gst_deinit() can only be run once per\nprocess live-time. So if you have multiple test in the same process, make sure\nthis happens only once.\n\n> +\t\tstatus_ = TestFail;\n> +\t\treturn;\n> +\t}\n> +\n> +\t/* Create the elements */\n> +\tlibcameraSrc_ = gst_element_factory_make(\"libcamerasrc\", \"libcamera\");\n> +\tpipeline_ = gst_pipeline_new(\"test-pipeline\");\n\nPersonally, I would entirely defer this, and create a virtual func to create the\npipeline. This way test can use gst_parse_launch() instead of handcrafting test\npipelines.\n\n> +\n> +\tif (!pipeline_ || !libcameraSrc_) {\n> +\t\tg_printerr(\"Not all elements could be created. %p.%p\\n\",\n> +\t\t\t pipeline_, libcameraSrc_);\n> +\t\tif (pipeline_)\n> +\t\t\tgst_object_unref(pipeline_);\n> +\t\tif (libcameraSrc_)\n> +\t\t\tgst_object_unref(libcameraSrc_);\nTo avoid this overhead, use local g_autoptr() and then blabla_ =\ng_steal_pointer() to save it.\n> +\t\tgst_deinit();\n> +\t\tstatus_ = TestFail;\n> +\t\treturn;\n> +\t}\n> +\n> +\tstatus_ = TestPass;\n\nI peaked at the usage, and I'm not big fan, have you considered adding an init()\nfunction with a proper return value? The C++ way is to use exception, not sure\nwhat the libcamera project thinks about it.\n\n> +}\n> +\n> +GstreamerTest::~GstreamerTest()\n> +{\n> +\tgst_object_unref(pipeline_);\n> +\tif (status_ == TestFail) {\n> +\t\tgst_object_unref(libcameraSrc_);\n> +\t}\n> +\n> +\tgst_deinit();\n\nAs you already do this here, and destructor get called regardless if the\nconstructor worked or not, I think ou can remove all the duplicates in the\nconstructor.\n\n> +}\n> +\n> +void GstreamerTest::gstreamer_start_pipeline()\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\tstatus_ = TestFail;\n> +\t\treturn;\n> +\t}\n> +\n> +\tstatus_ = TestPass;\n\nWhy not use a return value ?\n\n> +}\n> +\n> +void GstreamerTest::gstreamer_wait_for_event()\n> +{\n> +\t/* Wait until error or EOS or timeout after 2 seconds */\n> +\tconstexpr GstMessageType msgType =\n> +\t\tstatic_cast(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\nCan we make the msgType a optional argument here ? IIRC C++ allow for default\nvalue.\n\n> +\n> +\tgst_element_set_state(pipeline_, GST_STATE_NULL);\n\nThe method is called \"wait_for_event\", having it stop the pipeline is a bit\nunexpected.\n\n> +\n> +\t/* Parse error message */\n> +\tif (msg == NULL) {\n> +\t\tstatus_ = TestPass;\n> +\t\treturn;\n> +\t}\n> +\n> +\tswitch (GST_MESSAGE_TYPE(msg)) {\n> +\tcase GST_MESSAGE_ERROR:\n> +\t\tgstreamer_print_error(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> +\tstatus_ = TestPass;\n> +}\n> +\n> +void GstreamerTest::gstreamer_print_error(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> diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h\n> new file mode 100644\n> index 00000000..8e117166\n> --- /dev/null\n> +++ b/test/gstreamer/gstreamer_test.h\n> @@ -0,0 +1,38 @@\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 \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/source_paths.h\"\n> +\n> +#include \n> +\n> +using namespace std;\n> +\n> +class GstreamerTest\n> +{\n> +public:\n> +\tGstreamerTest();\n> +\t~GstreamerTest();\n> +\n> +protected:\n> +\tvoid gstreamer_start_pipeline();\n> +\tvoid gstreamer_wait_for_event();\n> +\tvoid gstreamer_print_error(GstMessage *msg);\n> +\n> +\tGstElement *pipeline_;\n> +\tGstElement *libcameraSrc_;\n> +\tint status_;\n> +};\n> +\n> +#endif /* __LIBCAMERA_GSTREAMER_TEST_H__ */\n> \\ No newline at end of file\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)","headers":{"Return-Path":"","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 060E8BDC71\n\tfor ;\n\tMon, 30 Aug 2021 20:36:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 386B36916A;\n\tMon, 30 Aug 2021 22:36:16 +0200 (CEST)","from mail-io1-xd35.google.com (mail-io1-xd35.google.com\n\t[IPv6:2607:f8b0:4864:20::d35])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BD4F60258\n\tfor ;\n\tMon, 30 Aug 2021 22:36:14 +0200 (CEST)","by mail-io1-xd35.google.com with SMTP id b200so21670317iof.13\n\tfor ;\n\tMon, 30 Aug 2021 13:36:14 -0700 (PDT)","from nicolas-tpx395.localdomain (mtl.collabora.ca. [66.171.169.34])\n\tby smtp.gmail.com with ESMTPSA id\n\te84sm8872928iof.21.2021.08.30.13.36.11\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 30 Aug 2021 13:36:12 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com\n\theader.b=\"dbcNx1QR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:date:in-reply-to:references:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=KPKR+ZiN/sOaEQJSDo4T4HQpG9/sxGBUTAxYUgB1IsI=;\n\tb=dbcNx1QRDMAQwR0w/dglMFF85iB7B3QAuNroES7+Ska1uLbvOu7jMqmV+vKFKU7uSK\n\t1uKw7big5dtzkVpbayn3q4UGE6+IHMA7mjjIVWsXWtSfE3h5eNmVw0/xFQh4EsqH3IAq\n\tYk7IHvXAyu72lT/hDwdDhY8lAxmura9/nLZVW+XXu1Zleq+AF8TEWfn6C4e8FMZ+zGYu\n\t12dfrx6kzZtnt6pXL4UnQpmqgcZ5Qq5a9AhizAEME0fqvpDegmq6+XYF2p8Y2YZ5cw+Z\n\t13HBX4pOaYyIx88Ny/apVUnxHqU4+aB8pHG9Hq4xemEvp8y9tNNDpQ3Xz7NUmTaOWAs6\n\t80xg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=KPKR+ZiN/sOaEQJSDo4T4HQpG9/sxGBUTAxYUgB1IsI=;\n\tb=n5eY/0FzePxp7Zvq+Y33Oh3n8Z9HMAIarq7ENf2Q8tsoY1hZrMGQIDaaDmCgBwD1vA\n\t/PUt7ElFJuM02z6+lB25NObhnxbnIG3S/0N7UfkyHQnCTJ+vLTG/FlnqWuh+j8t66Ovg\n\tLPzxwq0ufNKfHtU+kSFovoZsbgQLRcq6uTriw6OJwCYaMbtMh1zuAnCU9Qaf0y10J0TH\n\tksYd7E8ZZ8S7I+1k9hVUlxucWbUxpT5oVPn6lRa4sJYvCR8Ro8Q2I++Iszhh7nwzS0tP\n\tfbZ9+qyenEwzysV2iVKs4FJA2+TKxjtTWAQgFO7dzqLV+HxP56/3vByIMvS619G7uqSu\n\tiSHA==","X-Gm-Message-State":"AOAM532XgNB8VJwiA7tkvFmmSh7CtPkruX5U6Vvq1V89Doq/OckJyNVV\n\tLQu0ZUiHAzUj0gKTCb7BBoInQw==","X-Google-Smtp-Source":"ABdhPJwgcXyeF/riY3wlbftejscKL6AdcXhrsV2k29klbQILwBFd4R0amehJf/jY78UzNotqPAhHnw==","X-Received":"by 2002:a5d:8d1a:: with SMTP id\n\tp26mr5010083ioj.141.1630355773025; \n\tMon, 30 Aug 2021 13:36:13 -0700 (PDT)","Message-ID":"<969940441892639c57d95f77d96d7a4a5d29dcf3.camel@ndufresne.ca>","From":"Nicolas Dufresne ","To":"Vedant Paranjape , \n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 30 Aug 2021 16:36:10 -0400","In-Reply-To":"<20210829210632.432207-1-vedantparanjape160201@gmail.com>","References":"<20210829210632.432207-1-vedantparanjape160201@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v1] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19186,"web_url":"https://patchwork.libcamera.org/comment/19186/","msgid":"","date":"2021-08-30T21:09:05","subject":"Re: [libcamera-devel] [PATCH v1] test: gstreamer: Factor out code\n\tinto a base class","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hi Nicolas,\nThanks for the review.\n\n\nOn Tue, Aug 31, 2021 at 2:06 AM Nicolas Dufresne \nwrote:\n\n> Thanks for the patch,\n>\n> see inline comment for possible improvements.\n>\n> Le lundi 30 août 2021 à 02:36 +0530, Vedant Paranjape a écrit :\n> > Lot of code used in single stream test is boiler plate and common across\n> > every gstreamer test. Factored out this code into a base class called\n> > GstreamerTest.\n> >\n> > Also updated the gstreamer_single_stream_test to use the GstreamerTest\n> > base class\n> >\n> > Signed-off-by: Vedant Paranjape \n> > ---\n> > .../gstreamer_single_stream_test.cpp | 138 +++------------\n> > test/gstreamer/gstreamer_test.cpp | 157 ++++++++++++++++++\n> > test/gstreamer/gstreamer_test.h | 38 +++++\n> > test/gstreamer/meson.build | 2 +-\n> > 4 files changed, 216 insertions(+), 119 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\n> b/test/gstreamer/gstreamer_single_stream_test.cpp\n> > index 4c8d4804..021ac269 100644\n> > --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> > +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> > @@ -14,86 +14,35 @@\n> >\n> > #include \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> > - /*\n> > - * Disable leak detection due to a known global variable\n> initialization\n> > - * leak in glib's g_quark_init(). This should ideally be handled by\n> > - * using a suppression file instead of disabling leak detection.\n> > - */\n> > - return \"detect_leaks=false\";\n> > -}\n> > -}\n> > -\n> > -class GstreamerSingleStreamTest : public Test\n> > +class GstreamerSingleStreamTest : public GstreamerTest, public Test\n> > {\n> > +public:\n> > + GstreamerSingleStreamTest()\n> > + : GstreamerTest()\n> > + {\n> > + }\n> > +\n> > protected:\n> > int init() override\n> > {\n> > - /*\n> > - * GStreamer by default spawns a process to run the\n> > - * gst-plugin-scanner helper. If libcamera is compiled\n> with ASan\n> > - * enabled, and as GStreamer is most likely not, this\n> causes the\n> > - * ASan link order check to fail when gst-plugin-scanner\n> > - * dlopen()s the plugin as many libraries will have\n> already been\n> > - * loaded by then. Fix this issue by disabling spawning of\n> a\n> > - * child helper process when scanning the build directory\n> for\n> > - * plugins.\n> > - */\n> > - gst_registry_fork_set_enabled(false);\n> > -\n> > - /* Initialize GStreamer */\n> > - g_autoptr(GError) errInit = NULL;\n> > - if (!gst_init_check(nullptr, nullptr, &errInit)) {\n> > - g_printerr(\"Could not initialize GStreamer: %s\\n\",\n> > - errInit ? errInit->message : \"unknown\n> error\");\n> > -\n> > - return TestFail;\n> > - }\n> > -\n> > - /*\n> > - * Remove the system libcamera plugin, if any, and add the\n> > - * plugin from the build directory.\n> > - */\n> > - GstRegistry *registry = gst_registry_get();\n> > - GstPlugin *plugin = gst_registry_lookup(registry,\n> \"libgstlibcamera.so\");\n> > - if (plugin) {\n> > - gst_registry_remove_plugin(registry, plugin);\n> > - gst_object_unref(plugin);\n> > - }\n> > -\n> > - std::string path = libcamera::utils::libcameraBuildPath()\n> > - + \"src/gstreamer\";\n> > - if (!gst_registry_scan_path(registry, path.c_str())) {\n> > - g_printerr(\"Failed to add plugin to registry\\n\");\n> > - gst_deinit();\n> > - return TestFail;\n> > - }\n> > + if (status_ != TestPass)\n> > + return status_;\n> >\n> > - /* Create the elements */\n> > - libcameraSrc_ = gst_element_factory_make(\"libcamerasrc\",\n> \"libcamera\");\n> > convert0_ = gst_element_factory_make(\"videoconvert\",\n> \"convert0\");\n> > sink0_ = gst_element_factory_make(\"fakesink\", \"sink0\");\n> >\n> > - /* Create the empty pipeline_ */\n> > - pipeline_ = gst_pipeline_new(\"test-pipeline\");\n> > -\n> > - if (!pipeline_ || !convert0_ || !sink0_ || !libcameraSrc_)\n> {\n> > - g_printerr(\"Not all elements could be created.\n> %p.%p.%p.%p\\n\",\n> > - pipeline_, convert0_, sink0_,\n> libcameraSrc_);\n> > - if (pipeline_)\n> > - gst_object_unref(pipeline_);\n> > + if (!convert0_ || !sink0_) {\n> > + g_printerr(\"Not all elements could be created.\n> %p.%p\\n\",\n> > + convert0_, sink0_);\n> > if (convert0_)\n> > gst_object_unref(convert0_);\n> > if (sink0_)\n> > gst_object_unref(sink0_);\n> > - if (libcameraSrc_)\n> > - gst_object_unref(libcameraSrc_);\n> > gst_deinit();\n> >\n> > return TestFail;\n> > @@ -102,16 +51,8 @@ protected:\n> > return TestPass;\n> > }\n> >\n> > - void cleanup() override\n> > - {\n> > - gst_object_unref(pipeline_);\n> > - gst_deinit();\n> > - }\n> > -\n> > int run() override\n> > {\n> > - GstStateChangeReturn ret;\n> > -\n> > /* Build the pipeline */\n> > gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_,\n> convert0_, sink0_, NULL);\n> > if (gst_element_link_many(libcameraSrc_, convert0_,\n> sink0_, NULL) != TRUE) {\n> > @@ -119,57 +60,18 @@ protected:\n> > return TestFail;\n> > }\n> >\n> > - /* Start playing */\n> > - ret = gst_element_set_state(pipeline_, GST_STATE_PLAYING);\n> > - if (ret == GST_STATE_CHANGE_FAILURE) {\n> > - g_printerr(\"Unable to set the pipeline to the\n> playing state.\\n\");\n> > - return TestFail;\n> > - }\n> > + gstreamer_start_pipeline();\n> > + if (status_ != TestPass)\n> > + return status_;\n> >\n> > - /* Wait until error or EOS or timeout after 2 seconds */\n> > - constexpr GstMessageType msgType =\n> > - static_cast(GST_MESSAGE_ERROR |\n> GST_MESSAGE_EOS);\n> > - constexpr GstClockTime timeout = 2 * GST_SECOND;\n> > -\n> > - g_autoptr(GstBus) bus = gst_element_get_bus(pipeline_);\n> > - g_autoptr(GstMessage) msg =\n> gst_bus_timed_pop_filtered(bus, timeout, msgType);\n> > -\n> > - gst_element_set_state(pipeline_, GST_STATE_NULL);\n> > -\n> > - /* Parse error message */\n> > - if (msg == NULL)\n> > - return TestPass;\n> > -\n> > - switch (GST_MESSAGE_TYPE(msg)) {\n> > - case GST_MESSAGE_ERROR:\n> > - gstreamer_print_error(msg);\n> > - break;\n> > - case GST_MESSAGE_EOS:\n> > - g_print(\"End-Of-Stream reached.\\n\");\n> > - break;\n> > - default:\n> > - g_printerr(\"Unexpected message received.\\n\");\n> > - break;\n> > - }\n> > + gstreamer_wait_for_event();\n> > + if (status_ != TestPass)\n> > + return status_;\n> >\n> > - return TestFail;\n> > + return TestPass;\n> > }\n> >\n> > private:\n> > - void gstreamer_print_error(GstMessage *msg)\n> > - {\n> > - g_autoptr(GError) err = NULL;\n> > - g_autofree gchar *debug_info = NULL;\n> > -\n> > - gst_message_parse_error(msg, &err, &debug_info);\n> > - g_printerr(\"Error received from element %s: %s\\n\",\n> > - GST_OBJECT_NAME(msg->src), err->message);\n> > - g_printerr(\"Debugging information: %s\\n\",\n> > - debug_info ? debug_info : \"none\");\n> > - }\n> > -\n> > - GstElement *pipeline_;\n> > - GstElement *libcameraSrc_;\n> > GstElement *convert0_;\n> > GstElement *sink0_;\n> > };\n> > diff --git a/test/gstreamer/gstreamer_test.cpp\n> b/test/gstreamer/gstreamer_test.cpp\n> > new file mode 100644\n> > index 00000000..34608a02\n> > --- /dev/null\n> > +++ b/test/gstreamer/gstreamer_test.cpp\n> > @@ -0,0 +1,157 @@\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> > + /*\n> > + * Disable leak detection due to a known global variable\n> initialization\n> > + * leak in glib's g_quark_init(). This should ideally be handled by\n> > + * using a suppression file instead of disabling leak detection.\n> > + */\n> > + return \"detect_leaks=false\";\n> > +}\n> > +}\n> > +\n> > +GstreamerTest::GstreamerTest()\n> > +{\n> > + /*\n> > + * GStreamer by default spawns a process to run the\n> > + * gst-plugin-scanner helper. If libcamera is compiled with ASan\n> > + * enabled, and as GStreamer is most likely not, this causes the\n> > + * ASan link order check to fail when gst-plugin-scanner\n> > + * dlopen()s the plugin as many libraries will have already been\n> > + * loaded by then. Fix this issue by disabling spawning of a\n> > + * child helper process when scanning the build directory for\n> > + * plugins.\n> > + */\n> > + gst_registry_fork_set_enabled(false);\n> > +\n> > + /* Initialize GStreamer */\n> > + g_autoptr(GError) errInit = NULL;\n> > + if (!gst_init_check(nullptr, nullptr, &errInit)) {\n> > + g_printerr(\"Could not initialize GStreamer: %s\\n\",\n> > + errInit ? errInit->message : \"unknown error\");\n> > +\n> > + status_ = TestFail;\n> > + return;\n> > + }\n> > +\n> > + /*\n> > + * Remove the system libcamera plugin, if any, and add the\n> > + * plugin from the build directory.\n> > + */\n>\n> Indent looks wrong, did you enable the commit hooks ?\n>\n\nTypo on my side. Fixed.\n\n> + GstRegistry *registry = gst_registry_get();\n> > + GstPlugin *plugin = gst_registry_lookup(registry,\n> \"libgstlibcamera.so\");\n> > + if (plugin) {\n> > + gst_registry_remove_plugin(registry, plugin);\n> > + gst_object_unref(plugin);\n>\n> nit: Perhaps use a g_autoptr() instead ?\n>\n> > + }\n> > +\n> > + std::string path = libcamera::utils::libcameraBuildPath() +\n> \"src/gstreamer\";\n> > + if (!gst_registry_scan_path(registry, path.c_str())) {\n> > + g_printerr(\"Failed to add plugin to registry\\n\");\n> > + gst_deinit();\n>\n> Move this in the destructor ? Note that gst_deinit() can only be run once\n> per\n> process live-time. So if you have multiple test in the same process, make\n> sure\n> this happens only once.\n>\n> > + status_ = TestFail;\n> > + return;\n> > + }\n> > +\n> > + /* Create the elements */\n> > + libcameraSrc_ = gst_element_factory_make(\"libcamerasrc\",\n> \"libcamera\");\n> > + pipeline_ = gst_pipeline_new(\"test-pipeline\");\n>\n> Personally, I would entirely defer this, and create a virtual func to\n> create the\n> pipeline. This way test can use gst_parse_launch() instead of handcrafting\n> test\n> pipelines.\n>\n> +\n> > + if (!pipeline_ || !libcameraSrc_) {\n> > + g_printerr(\"Not all elements could be created. %p.%p\\n\",\n> > + pipeline_, libcameraSrc_);\n> > + if (pipeline_)\n> > + gst_object_unref(pipeline_);\n> > + if (libcameraSrc_)\n> > + gst_object_unref(libcameraSrc_);\n> To avoid this overhead, use local g_autoptr() and then blabla_ =\n> g_steal_pointer() to save it.\n> > + gst_deinit();\n> > + status_ = TestFail;\n> > + return;\n> > + }\n> > +\n> > + status_ = TestPass;\n>\n> I peaked at the usage, and I'm not big fan, have you considered adding an\n> init()\n> function with a proper return value? The C++ way is to use exception, not\n> sure\n> what the libcamera project thinks about it.\n>\n\nJust did what other base test classes did.\n\n> +}\n> > +\n> > +GstreamerTest::~GstreamerTest()\n> > +{\n> > + gst_object_unref(pipeline_);\n> > + if (status_ == TestFail) {\n> > + gst_object_unref(libcameraSrc_);\n> > + }\n> > +\n> > + gst_deinit();\n>\n> As you already do this here, and destructor get called regardless if the\n> constructor worked or not, I think ou can remove all the duplicates in the\n> constructor.\n>\n> > +}\n> > +\n> > +void GstreamerTest::gstreamer_start_pipeline()\n> > +{\n> > + GstStateChangeReturn ret;\n> > +\n> > + /* Start playing */\n> > + ret = gst_element_set_state(pipeline_, GST_STATE_PLAYING);\n> > + if (ret == GST_STATE_CHANGE_FAILURE) {\n> > + g_printerr(\"Unable to set the pipeline to the playing\n> state.\\n\");\n> > + status_ = TestFail;\n> > + return;\n> > + }\n> > +\n> > + status_ = TestPass;\n>\n> Why not use a return value ?\n>\n\nNice catch, fixed it !\n\n> +}\n> > +\n> > +void GstreamerTest::gstreamer_wait_for_event()\n> > +{\n> > + /* Wait until error or EOS or timeout after 2 seconds */\n> > + constexpr GstMessageType msgType =\n> > + static_cast(GST_MESSAGE_ERROR |\n> GST_MESSAGE_EOS);\n> > + constexpr GstClockTime timeout = 2 * GST_SECOND;\n> > +\n> > + g_autoptr(GstBus) bus = gst_element_get_bus(pipeline_);\n> > + g_autoptr(GstMessage) msg = gst_bus_timed_pop_filtered(bus,\n> timeout, msgType);\n>\n> Can we make the msgType a optional argument here ? IIRC C++ allow for\n> default\n> value.\n>\n\n Not sure how to do it, since I am not defining a function.\n\n> +\n> > + gst_element_set_state(pipeline_, GST_STATE_NULL);\n>\n> The method is called \"wait_for_event\", having it stop the pipeline is a bit\n> unexpected.\n>\n\n\"process_event\" sounds much better ?\n\n> +\n> > + /* Parse error message */\n> > + if (msg == NULL) {\n> > + status_ = TestPass;\n> > + return;\n> > + }\n> > +\n> > + switch (GST_MESSAGE_TYPE(msg)) {\n> > + case GST_MESSAGE_ERROR:\n> > + gstreamer_print_error(msg);\n> > + break;\n> > + case GST_MESSAGE_EOS:\n> > + g_print(\"End-Of-Stream reached.\\n\");\n> > + break;\n> > + default:\n> > + g_printerr(\"Unexpected message received.\\n\");\n> > + break;\n> > + }\n> > +\n> > + status_ = TestPass;\n> > +}\n> > +\n> > +void GstreamerTest::gstreamer_print_error(GstMessage *msg)\n> > +{\n> > + g_autoptr(GError) err = NULL;\n> > + g_autofree gchar *debug_info = NULL;\n> > +\n> > + gst_message_parse_error(msg, &err, &debug_info);\n> > + g_printerr(\"Error received from element %s: %s\\n\",\n> > + GST_OBJECT_NAME(msg->src), err->message);\n> > + g_printerr(\"Debugging information: %s\\n\",\n> > + debug_info ? debug_info : \"none\");\n> > +}\n> > +\n> > diff --git a/test/gstreamer/gstreamer_test.h\n> b/test/gstreamer/gstreamer_test.h\n> > new file mode 100644\n> > index 00000000..8e117166\n> > --- /dev/null\n> > +++ b/test/gstreamer/gstreamer_test.h\n> > @@ -0,0 +1,38 @@\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 \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/source_paths.h\"\n> > +\n> > +#include \n> > +\n> > +using namespace std;\n> > +\n> > +class GstreamerTest\n> > +{\n> > +public:\n> > + GstreamerTest();\n> > + ~GstreamerTest();\n> > +\n> > +protected:\n> > + void gstreamer_start_pipeline();\n> > + void gstreamer_wait_for_event();\n> > + void gstreamer_print_error(GstMessage *msg);\n> > +\n> > + GstElement *pipeline_;\n> > + GstElement *libcameraSrc_;\n> > + int status_;\n> > +};\n> > +\n> > +#endif /* __LIBCAMERA_GSTREAMER_TEST_H__ */\n> > \\ No newline at end of file\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>\n>\nRegards,\n*Vedant Paranjape*","headers":{"Return-Path":"","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 D14FBBDC71\n\tfor ;\n\tMon, 30 Aug 2021 21:09:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2FC7969169;\n\tMon, 30 Aug 2021 23:09:21 +0200 (CEST)","from mail-yb1-xb2e.google.com (mail-yb1-xb2e.google.com\n\t[IPv6:2607:f8b0:4864:20::b2e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22E9660258\n\tfor ;\n\tMon, 30 Aug 2021 23:09:19 +0200 (CEST)","by mail-yb1-xb2e.google.com with SMTP id k65so30789034yba.13\n\tfor ;\n\tMon, 30 Aug 2021 14:09:19 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"AmqId5dr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=HvYxSGGZA8YrhACLC+zBK/O4qi1gaZsLMkOjpYhQhIM=;\n\tb=AmqId5droTZP30zD5qCmzAXbBNe9gU+PLvshfXEsLl7PtgleufunreNNOtQhRVQ7ZV\n\tCfGWetO8YmCRa6plXwJOJu29WGstBQFafl12cmUTlWIfoRs976EwUN00pnyJW1CDY6td\n\t21wdrxhycatfHVfG+FgLN/L/qiHDMIe8hElPQRQNH4XS6xqfg9PHzGaovnNB7tJrvsJd\n\tByv2tlSshzh3hB51eXAHJRczdtQj0iH0w+PjVzfGeWrHGNeiU5rmt0s4gmA2owFUb6iK\n\tRTzP+qAYCQHnJsF36z4ZA6z+N6h+fB5MoLW+IBO31sAU5rbGZaA6Vh+IeD7LmgSCAkWl\n\tN3WQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=HvYxSGGZA8YrhACLC+zBK/O4qi1gaZsLMkOjpYhQhIM=;\n\tb=h9opBZywfcnj5G/Fm7oRTZLaBkUsq2BsNeZK3pcBXKQQnBIxoSnR5xHw73Ti+qoVPR\n\t2s38f9JDAVsL0MGXeHxf3xai3b30t7uLnP0u6aN/OO6TLpcLAyZb06rHgRRMNyFJEzNr\n\tX/i2q0gRGgHQPrBvTW7T8PbxcS++FOI8bI0mhg/tL9O8hXc6qERt75Fq3l+ajwRtNKV0\n\t7QwJzW7HtJZdSJpSnq/RDFX1+7komrKsFl5cl/iamPeiJCA3NDmpPBkJX/KoVwKseFkL\n\tSS9p9COn8PoUN/oYO0No2OTvyYcibhnzwxUjzTMHCwyHTyd7cwxhY/0tVm6JBYvm393y\n\tAWpg==","X-Gm-Message-State":"AOAM530MvwRZ/iwPo2ckQ++BFfIPYlDWMawnmu23tyeu7a3kkqcY6C1a\n\tFI6cYXGfvZk8VysMTRGk2iouhWj0VSmGDxuhLhs=","X-Google-Smtp-Source":"ABdhPJyyxWvRTkeBPjin+zxUNyQJTFkA73Klzi+8RxPnpBRYK71lh3wTam/fFH9DKd1KWt5MF4t2brOIHRwhDztu/48=","X-Received":"by 2002:a25:99c8:: with SMTP id q8mr28471733ybo.63.1630357757719;\n\tMon, 30 Aug 2021 14:09:17 -0700 (PDT)","MIME-Version":"1.0","References":"<20210829210632.432207-1-vedantparanjape160201@gmail.com>\n\t<969940441892639c57d95f77d96d7a4a5d29dcf3.camel@ndufresne.ca>","In-Reply-To":"<969940441892639c57d95f77d96d7a4a5d29dcf3.camel@ndufresne.ca>","From":"Vedant Paranjape ","Date":"Tue, 31 Aug 2021 02:39:05 +0530","Message-ID":"","To":"Nicolas Dufresne ","Content-Type":"multipart/alternative; boundary=\"00000000000001792705cacd3f28\"","Subject":"Re: [libcamera-devel] [PATCH v1] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]