[{"id":29503,"web_url":"https://patchwork.libcamera.org/comment/29503/","msgid":"<171526621889.1857112.1496126054128483482@ping.linuxembedded.co.uk>","date":"2024-05-09T14:50:18","subject":"Re: [PATCH v2 2/3] test: gstreamer: Simplify single stream test","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Nicolas Dufresne (2024-03-05 15:30:57)\n> From: Nicolas Dufresne \n> \n\nWhy? Is it just better?\n\nI'll try to interpret as I read the patch. Please correct me whereever\nI'm wrong.\n\n\"\"\"\nThe single stream test for the gstreamer component has a simple pipeline\nconstruction using only a fakesink.\n\nThe impelementation currently supports connecting to a more complex\nstream construction defined by the streamDescription, but this is over\nengineered for the simple need to start a stream to capture and discard\nthe frames.\n\nConvert the use of gst_parse_bin_from_description_full() which uses only\na single element 'fakesink' to construct the fakesink directly and link\nit to the libcamerasrc.\n\"\"\"\n\n> Signed-off-by: Nicolas Dufresne \n\n\nReviewed-by: Kieran Bingham \n\n> ---\n> .../gstreamer_single_stream_test.cpp | 27 +++++++------------\n> 1 file changed, 9 insertions(+), 18 deletions(-)\n> \n> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp\n> index 301e4a93..815c6609 100644\n> --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> @@ -29,30 +29,21 @@ protected:\n> if (status_ != TestPass)\n> return status_;\n> \n> - const gchar *streamDescription = \"fakesink\";\n> - g_autoptr(GError) error0 = NULL;\n> - stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> - NULL,\n> - GST_PARSE_FLAG_FATAL_ERRORS,\n> - &error0);\n> -\n> - if (!stream0_) {\n> - g_printerr(\"Bin could not be created (%s)\\n\", error0->message);\n> + fakesink_ = gst_element_factory_make(\"fakesink\", nullptr);\n> + if (!fakesink_) {\n> + g_printerr(\"Your installation is missing 'fakesink'\\n\");\n> return TestFail;\n> }\n> - g_object_ref_sink(stream0_);\n> -\n> - if (createPipeline() != TestPass)\n> - return TestFail;\n> + g_object_ref_sink(fakesink_);\n> \n> - return TestPass;\n> + return createPipeline();\n> }\n> \n> int run() override\n> {\n> /* Build the pipeline */\n> - gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, stream0_, NULL);\n> - if (gst_element_link(libcameraSrc_, stream0_) != TRUE) {\n> + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, fakesink_, nullptr);\n> + if (!gst_element_link(libcameraSrc_, fakesink_)) {\n> g_printerr(\"Elements could not be linked.\\n\");\n> return TestFail;\n> }\n> @@ -68,11 +59,11 @@ protected:\n> \n> void cleanup() override\n> {\n> - g_clear_object(&stream0_);\n> + g_clear_object(&fakesink_);\n> }\n> \n> private:\n> - GstElement *stream0_;\n> + GstElement *fakesink_;\n> };\n> \n> TEST_REGISTER(GstreamerSingleStreamTest)\n> -- \n> 2.43.2\n>","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 4C75EBDE6B\n\tfor ;\n\tThu, 9 May 2024 14:50:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3476063433;\n\tThu, 9 May 2024 16:50:23 +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 BEE19633FA\n\tfor ;\n\tThu, 9 May 2024 16:50:21 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1A1FC3D5;\n\tThu, 9 May 2024 16:50:18 +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=\"P48g1MWi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715266218;\n\tbh=MTFl2MyIWs6aGn+KSAN7ITHcjkeIXCB+0IGGHpxPdmI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=P48g1MWierdjQauQ/KHPMreAmOGHWQaeilq5KSLt+gUUcHIsJFZxhJI+N1//OGQ41\n\tT5dkyd3Zxk2tPg7CNL2QM7v8LoxARsQoTkyNEU9QROqHaQrPukFGdBHxsZUFf4Of78\n\tx6Lv1sdcr0o1Oy322l6dWo6cdDll+ubdIohsMDz8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240305153058.1761020-3-nicolas@ndufresne.ca>","References":"<20240305153058.1761020-1-nicolas@ndufresne.ca>\n\t<20240305153058.1761020-3-nicolas@ndufresne.ca>","Subject":"Re: [PATCH v2 2/3] test: gstreamer: Simplify single stream test","From":"Kieran Bingham ","Cc":"Nicolas Dufresne ","To":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 09 May 2024 15:50:18 +0100","Message-ID":"<171526621889.1857112.1496126054128483482@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":29506,"web_url":"https://patchwork.libcamera.org/comment/29506/","msgid":"","date":"2024-05-09T15:12:18","subject":"Re: [PATCH v2 2/3] test: gstreamer: Simplify single stream test","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Hi,\n\nLe jeudi 09 mai 2024 à 15:50 +0100, Kieran Bingham a écrit :\n> Quoting Nicolas Dufresne (2024-03-05 15:30:57)\n> > From: Nicolas Dufresne \n> > \n> \n> Why? Is it just better?\n\nI commit and sign with my Collabora address for all the work I do on my employer\ntime. But I'm using my personal address for mailing lists. I did no action here,\n\"git send-email\" noticed and added the from. If you use \"git am\". it should\ntransparently reflect my choice here.\n\n> \n> I'll try to interpret as I read the patch. Please correct me whereever\n> I'm wrong.\n> \n> \"\"\"\n> The single stream test for the gstreamer component has a simple pipeline\n> construction using only a fakesink.\n> \n> The impelementation currently supports connecting to a more complex\n> stream construction defined by the streamDescription, but this is over\n> engineered for the simple need to start a stream to capture and discard\n> the frames.\n> \n> Convert the use of gst_parse_bin_from_description_full() which uses only\n> a single element 'fakesink' to construct the fakesink directly and link\n> it to the libcamerasrc.\n> \"\"\"\n\nThat is very good, I should have wrote something, but if the target was the\nGStreamer project, it would have been considered overkill due to the trivality\nof the changes (from a GStreamer dev point of view of course). I'll keep in mind\nfor next time.\n\n> \n> > Signed-off-by: Nicolas Dufresne \n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > ---\n> > .../gstreamer_single_stream_test.cpp | 27 +++++++------------\n> > 1 file changed, 9 insertions(+), 18 deletions(-)\n> > \n> > diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp\n> > index 301e4a93..815c6609 100644\n> > --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> > +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> > @@ -29,30 +29,21 @@ protected:\n> > if (status_ != TestPass)\n> > return status_;\n> > \n> > - const gchar *streamDescription = \"fakesink\";\n> > - g_autoptr(GError) error0 = NULL;\n> > - stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> > - NULL,\n> > - GST_PARSE_FLAG_FATAL_ERRORS,\n> > - &error0);\n> > -\n> > - if (!stream0_) {\n> > - g_printerr(\"Bin could not be created (%s)\\n\", error0->message);\n> > + fakesink_ = gst_element_factory_make(\"fakesink\", nullptr);\n> > + if (!fakesink_) {\n> > + g_printerr(\"Your installation is missing 'fakesink'\\n\");\n> > return TestFail;\n> > }\n> > - g_object_ref_sink(stream0_);\n> > -\n> > - if (createPipeline() != TestPass)\n> > - return TestFail;\n> > + g_object_ref_sink(fakesink_);\n> > \n> > - return TestPass;\n> > + return createPipeline();\n> > }\n> > \n> > int run() override\n> > {\n> > /* Build the pipeline */\n> > - gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, stream0_, NULL);\n> > - if (gst_element_link(libcameraSrc_, stream0_) != TRUE) {\n> > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, fakesink_, nullptr);\n> > + if (!gst_element_link(libcameraSrc_, fakesink_)) {\n> > g_printerr(\"Elements could not be linked.\\n\");\n> > return TestFail;\n> > }\n> > @@ -68,11 +59,11 @@ protected:\n> > \n> > void cleanup() override\n> > {\n> > - g_clear_object(&stream0_);\n> > + g_clear_object(&fakesink_);\n> > }\n> > \n> > private:\n> > - GstElement *stream0_;\n> > + GstElement *fakesink_;\n> > };\n> > \n> > TEST_REGISTER(GstreamerSingleStreamTest)\n> > -- \n> > 2.43.2\n> >","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 2F44FC3226\n\tfor ;\n\tThu, 9 May 2024 15:12:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 474BE6342D;\n\tThu, 9 May 2024 17:12:25 +0200 (CEST)","from madrid.collaboradmins.com (madrid.collaboradmins.com\n\t[46.235.227.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEC90633FA\n\tfor ;\n\tThu, 9 May 2024 17:12:23 +0200 (CEST)","from nicolas-tpx395.localdomain (cola.collaboradmins.com\n\t[195.201.22.229])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby madrid.collaboradmins.com (Postfix) with ESMTPSA id 1F38637811CD; \n\tThu, 9 May 2024 15:12:23 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=collabora.com header.i=@collabora.com\n\theader.b=\"5kVTssE/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1715267543;\n\tbh=Uy5X/Lh0CAWxrq69XSCJkouMduZ44oM0h1px/JEJKE0=;\n\th=Subject:From:To:Date:In-Reply-To:References:From;\n\tb=5kVTssE/ECEca04z5HMlO0xq1zOPSUFklhNEU8MIoxg7rbam+BYHq693L3g0fIOv8\n\thtWA0QNAsqtC6bZZKlVrjPvIuMZhkumaNN2geGzF9VF/89bUxHRr9cHgnHWOO+inyd\n\tD7cV2zGztOGeTYJ5XRu6p3vcHAL6EJRqt5ZVhf+b5W8BDVpMhqab0gPjtn060L8Lml\n\tdNTnxGl7+BADoNJMjtl2RprcmtVgBpU5UZ3kSD5aAmmjrmT6GRCsDo2/RKj5a9squ5\n\t6l01y9JT68aYJQj7XeNq476QM0cgPutM9G/zLRxzRyuitIKGrh3orfibIabAgTGRoi\n\tW87A4QaYvg0FQ==","Message-ID":"","Subject":"Re: [PATCH v2 2/3] test: gstreamer: Simplify single stream test","From":"Nicolas Dufresne ","To":"Kieran Bingham , \n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 09 May 2024 11:12:18 -0400","In-Reply-To":"<171526621889.1857112.1496126054128483482@ping.linuxembedded.co.uk>","References":"<20240305153058.1761020-1-nicolas@ndufresne.ca>\n\t<20240305153058.1761020-3-nicolas@ndufresne.ca>\n\t<171526621889.1857112.1496126054128483482@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.52.1 (3.52.1-1.fc40) ","MIME-Version":"1.0","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":29510,"web_url":"https://patchwork.libcamera.org/comment/29510/","msgid":"<171527024991.75000.5845196870246259896@ping.linuxembedded.co.uk>","date":"2024-05-09T15:57:29","subject":"Re: [PATCH v2 2/3] test: gstreamer: Simplify single stream test","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Nicolas Dufresne (2024-05-09 16:12:18)\n> Hi,\n> \n> Le jeudi 09 mai 2024 à 15:50 +0100, Kieran Bingham a écrit :\n> > Quoting Nicolas Dufresne (2024-03-05 15:30:57)\n> > > From: Nicolas Dufresne \n> > > \n> > \n> > Why? Is it just better?\n> \n> I commit and sign with my Collabora address for all the work I do on my employer\n> time. But I'm using my personal address for mailing lists. I did no action here,\n> \"git send-email\" noticed and added the from. If you use \"git am\". it should\n> transparently reflect my choice here.\n\nSorry - I meant - the commit message was empty. I don't mind (or care)\nwhich the From is from ;-)\n\nI meant \"Simplify single stream test\" - Why - is it better simplified?\nThe empty commit message didn't tell us anything!\n\n> \n> > \n> > I'll try to interpret as I read the patch. Please correct me whereever\n> > I'm wrong.\n> > \n> > \"\"\"\n> > The single stream test for the gstreamer component has a simple pipeline\n> > construction using only a fakesink.\n> > \n> > The impelementation currently supports connecting to a more complex\n> > stream construction defined by the streamDescription, but this is over\n> > engineered for the simple need to start a stream to capture and discard\n> > the frames.\n> > \n> > Convert the use of gst_parse_bin_from_description_full() which uses only\n> > a single element 'fakesink' to construct the fakesink directly and link\n> > it to the libcamerasrc.\n> > \"\"\"\n> \n> That is very good, I should have wrote something, but if the target was the\n> GStreamer project, it would have been considered overkill due to the trivality\n> of the changes (from a GStreamer dev point of view of course). I'll keep in mind\n> for next time.\n\nThe difficulty for me is I'm not a gstreamer dev, so I find the\ngstreamer element difficult to read. An introduction is very helpful to\ngo into the patch with something in mind.\n\nIf you're happy with the commit message, it can be added at applying or\nin a v2 - I don't know if a v2 is needed yet.\n\n\n> \n> > \n> > > Signed-off-by: Nicolas Dufresne \n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > > ---\n> > > .../gstreamer_single_stream_test.cpp | 27 +++++++------------\n> > > 1 file changed, 9 insertions(+), 18 deletions(-)\n> > > \n> > > diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp\n> > > index 301e4a93..815c6609 100644\n> > > --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> > > +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> > > @@ -29,30 +29,21 @@ protected:\n> > > if (status_ != TestPass)\n> > > return status_;\n> > > \n> > > - const gchar *streamDescription = \"fakesink\";\n> > > - g_autoptr(GError) error0 = NULL;\n> > > - stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> > > - NULL,\n> > > - GST_PARSE_FLAG_FATAL_ERRORS,\n> > > - &error0);\n> > > -\n> > > - if (!stream0_) {\n> > > - g_printerr(\"Bin could not be created (%s)\\n\", error0->message);\n> > > + fakesink_ = gst_element_factory_make(\"fakesink\", nullptr);\n> > > + if (!fakesink_) {\n> > > + g_printerr(\"Your installation is missing 'fakesink'\\n\");\n> > > return TestFail;\n> > > }\n> > > - g_object_ref_sink(stream0_);\n> > > -\n> > > - if (createPipeline() != TestPass)\n> > > - return TestFail;\n> > > + g_object_ref_sink(fakesink_);\n> > > \n> > > - return TestPass;\n> > > + return createPipeline();\n> > > }\n> > > \n> > > int run() override\n> > > {\n> > > /* Build the pipeline */\n> > > - gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, stream0_, NULL);\n> > > - if (gst_element_link(libcameraSrc_, stream0_) != TRUE) {\n> > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, fakesink_, nullptr);\n> > > + if (!gst_element_link(libcameraSrc_, fakesink_)) {\n> > > g_printerr(\"Elements could not be linked.\\n\");\n> > > return TestFail;\n> > > }\n> > > @@ -68,11 +59,11 @@ protected:\n> > > \n> > > void cleanup() override\n> > > {\n> > > - g_clear_object(&stream0_);\n> > > + g_clear_object(&fakesink_);\n> > > }\n> > > \n> > > private:\n> > > - GstElement *stream0_;\n> > > + GstElement *fakesink_;\n> > > };\n> > > \n> > > TEST_REGISTER(GstreamerSingleStreamTest)\n> > > -- \n> > > 2.43.2\n> > > \n>","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 61B3DBDE6B\n\tfor ;\n\tThu, 9 May 2024 15:57:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B02A6345F;\n\tThu, 9 May 2024 17:57:34 +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 32CAD633FA\n\tfor ;\n\tThu, 9 May 2024 17:57:33 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D7E9B0B;\n\tThu, 9 May 2024 17:57:29 +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=\"Q9pfUdcq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715270249;\n\tbh=5X++m2st6O+HGd8WvOT9NKtHhP5Ayibe/qGKydK/zoo=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Q9pfUdcqHlJVpBZJQNIPSMxm/O+c+Qm622sypWy88VlIE3lsoB6WBTkJV9JteljxA\n\t85UTjocdXJX7ZSzWbl+QAsjOevllJ4s+yykOBSVREsih/uGTkefdRbsOMMDQIU3hRZ\n\tIgvKxU4N9VFg3IAPa6tncvNPBgi2KZi22a8SWoCY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20240305153058.1761020-1-nicolas@ndufresne.ca>\n\t<20240305153058.1761020-3-nicolas@ndufresne.ca>\n\t<171526621889.1857112.1496126054128483482@ping.linuxembedded.co.uk>\n\t","Subject":"Re: [PATCH v2 2/3] test: gstreamer: Simplify single stream test","From":"Kieran Bingham ","To":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 09 May 2024 16:57:29 +0100","Message-ID":"<171527024991.75000.5845196870246259896@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":29519,"web_url":"https://patchwork.libcamera.org/comment/29519/","msgid":"<20240513115619.GB18630@pendragon.ideasonboard.com>","date":"2024-05-13T11:56:19","subject":"Re: [PATCH v2 2/3] test: gstreamer: Simplify single stream test","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, May 09, 2024 at 04:57:29PM +0100, Kieran Bingham wrote:\n> Quoting Nicolas Dufresne (2024-05-09 16:12:18)\n> > Hi,\n> > \n> > Le jeudi 09 mai 2024 à 15:50 +0100, Kieran Bingham a écrit :\n> > > Quoting Nicolas Dufresne (2024-03-05 15:30:57)\n> > > > From: Nicolas Dufresne \n> > > > \n> > > \n> > > Why? Is it just better?\n> > \n> > I commit and sign with my Collabora address for all the work I do on my employer\n> > time. But I'm using my personal address for mailing lists. I did no action here,\n> > \"git send-email\" noticed and added the from. If you use \"git am\". it should\n> > transparently reflect my choice here.\n> \n> Sorry - I meant - the commit message was empty. I don't mind (or care)\n> which the From is from ;-)\n> \n> I meant \"Simplify single stream test\" - Why - is it better simplified?\n> The empty commit message didn't tell us anything!\n> \n> > > I'll try to interpret as I read the patch. Please correct me whereever\n> > > I'm wrong.\n> > > \n> > > \"\"\"\n> > > The single stream test for the gstreamer component has a simple pipeline\n> > > construction using only a fakesink.\n> > > \n> > > The impelementation currently supports connecting to a more complex\n\ns/impelementation/implementation/\n\nReviewed-by: Laurent Pinchart \n\n> > > stream construction defined by the streamDescription, but this is over\n> > > engineered for the simple need to start a stream to capture and discard\n> > > the frames.\n> > > \n> > > Convert the use of gst_parse_bin_from_description_full() which uses only\n> > > a single element 'fakesink' to construct the fakesink directly and link\n> > > it to the libcamerasrc.\n> > > \"\"\"\n> > \n> > That is very good, I should have wrote something, but if the target was the\n> > GStreamer project, it would have been considered overkill due to the trivality\n> > of the changes (from a GStreamer dev point of view of course). I'll keep in mind\n> > for next time.\n> \n> The difficulty for me is I'm not a gstreamer dev, so I find the\n> gstreamer element difficult to read. An introduction is very helpful to\n> go into the patch with something in mind.\n> \n> If you're happy with the commit message, it can be added at applying or\n> in a v2 - I don't know if a v2 is needed yet.\n\nI'll merge this patch already, independently from the rest of the\nseries.\n\n> > > > Signed-off-by: Nicolas Dufresne \n> > > \n> > > Reviewed-by: Kieran Bingham \n> > > \n> > > > ---\n> > > > .../gstreamer_single_stream_test.cpp | 27 +++++++------------\n> > > > 1 file changed, 9 insertions(+), 18 deletions(-)\n> > > > \n> > > > diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp\n> > > > index 301e4a93..815c6609 100644\n> > > > --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> > > > +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> > > > @@ -29,30 +29,21 @@ protected:\n> > > > if (status_ != TestPass)\n> > > > return status_;\n> > > > \n> > > > - const gchar *streamDescription = \"fakesink\";\n> > > > - g_autoptr(GError) error0 = NULL;\n> > > > - stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,\n> > > > - NULL,\n> > > > - GST_PARSE_FLAG_FATAL_ERRORS,\n> > > > - &error0);\n> > > > -\n> > > > - if (!stream0_) {\n> > > > - g_printerr(\"Bin could not be created (%s)\\n\", error0->message);\n> > > > + fakesink_ = gst_element_factory_make(\"fakesink\", nullptr);\n> > > > + if (!fakesink_) {\n> > > > + g_printerr(\"Your installation is missing 'fakesink'\\n\");\n> > > > return TestFail;\n> > > > }\n> > > > - g_object_ref_sink(stream0_);\n> > > > -\n> > > > - if (createPipeline() != TestPass)\n> > > > - return TestFail;\n> > > > + g_object_ref_sink(fakesink_);\n> > > > \n> > > > - return TestPass;\n> > > > + return createPipeline();\n> > > > }\n> > > > \n> > > > int run() override\n> > > > {\n> > > > /* Build the pipeline */\n> > > > - gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, stream0_, NULL);\n> > > > - if (gst_element_link(libcameraSrc_, stream0_) != TRUE) {\n> > > > + gst_bin_add_many(GST_BIN(pipeline_), libcameraSrc_, fakesink_, nullptr);\n> > > > + if (!gst_element_link(libcameraSrc_, fakesink_)) {\n> > > > g_printerr(\"Elements could not be linked.\\n\");\n> > > > return TestFail;\n> > > > }\n> > > > @@ -68,11 +59,11 @@ protected:\n> > > > \n> > > > void cleanup() override\n> > > > {\n> > > > - g_clear_object(&stream0_);\n> > > > + g_clear_object(&fakesink_);\n> > > > }\n> > > > \n> > > > private:\n> > > > - GstElement *stream0_;\n> > > > + GstElement *fakesink_;\n> > > > };\n> > > > \n> > > > TEST_REGISTER(GstreamerSingleStreamTest)","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 5254BC3226\n\tfor ;\n\tMon, 13 May 2024 11:56:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3318C6346B;\n\tMon, 13 May 2024 13:56:29 +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 D27B861A5E\n\tfor ;\n\tMon, 13 May 2024 13:56:27 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 28C09AF3;\n\tMon, 13 May 2024 13:56:21 +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=\"UFaKcVIM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715601381;\n\tbh=mhbpJG0BHB11z30ftn8/0dHA8JWXcD7Ipoitp+rdSPA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UFaKcVIMTr0t5+D94J2HmC1BEgnP27yPf8TvHaz6url3nIhCgFbiabWH/TevIs1Dt\n\tTDNBDAGXhdqKiwxStiAz2tkD+OqOzJ9dJ8pDnjkGJAysBG98LFImSDuZVel4+T97qb\n\tNWoSrjwA6TGqM2wyVKMMLdLmumiqmu+cLk7iFkXg=","Date":"Mon, 13 May 2024 14:56:19 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 2/3] test: gstreamer: Simplify single stream test","Message-ID":"<20240513115619.GB18630@pendragon.ideasonboard.com>","References":"<20240305153058.1761020-1-nicolas@ndufresne.ca>\n\t<20240305153058.1761020-3-nicolas@ndufresne.ca>\n\t<171526621889.1857112.1496126054128483482@ping.linuxembedded.co.uk>\n\t\n\t<171527024991.75000.5845196870246259896@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<171527024991.75000.5845196870246259896@ping.linuxembedded.co.uk>","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\" "}}]