[{"id":19682,"web_url":"https://patchwork.libcamera.org/comment/19682/","msgid":"","date":"2021-09-14T17:43:38","subject":"Re: [libcamera-devel] [PATCH 1/2] test: gstreamer: Simplify\n\telements' ownerships","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le mardi 14 septembre 2021 à 17:46 +0530, Umang Jain a écrit :\n> In gstreamer, when elements are created, usually a floating [1]\n> reference is returned which simply means, there is no ownership\n> transfer (yet). Once can simply check for NULL and return through\n> an error path, without bothering to clean up. Hence, g_autoptr is\n> not much of help here.\n> \n> If the NULL checks have been passed successfully, elements are ready\n> to use. However, we must claim ownership/reference it before using\n> them via g_object_ref_sink().\n> \n> This patch build upon this principle and removes the g_autoptr\n> from gstreamer tests whereever necessary to tide up the code.\n> \n> [1] https://gstreamer.freedesktop.org/documentation/additional/design/MT-refcounting.html?gi-language=c#refcounting1\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Nicolas Dufresne \n\nNote, I still made a comment below ....\n\n> ---\n> test/gstreamer/gstreamer_single_stream_test.cpp | 14 ++++++--------\n> test/gstreamer/gstreamer_test.cpp | 9 ++++-----\n> 2 files changed, 10 insertions(+), 13 deletions(-)\n> \n> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp\n> index 7292f328..d992455c 100644\n> --- a/test/gstreamer/gstreamer_single_stream_test.cpp\n> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp\n> @@ -33,20 +33,18 @@ protected:\n> \t\tif (status_ != TestPass)\n> \t\t\treturn status_;\n> \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> +\t\tconvert0_ = gst_element_factory_make(\"videoconvert\", \"convert0\");\n> +\t\tsink0_ = gst_element_factory_make(\"fakesink\", \"sink0\");\n> \n> -\t\tif (!convert0 || !sink0) {\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\t\t convert0_, sink0_);\n> \n> \t\t\treturn TestFail;\n> \t\t}\n> \n> -\t\tconvert0_ = reinterpret_cast(g_steal_pointer(&convert0));\n> -\t\tsink0_ = reinterpret_cast(g_steal_pointer(&sink0));\n> +\t\tg_object_ref_sink(convert0_);\n> +\t\tg_object_ref_sink(sink0_);\n\nI prefer when ref_sink are inline or close to their allocation.\n\n> \n> \t\tif (createPipeline() != TestPass)\n> \t\t\treturn TestFail;\n> diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp\n> index 41712505..227a5c37 100644\n> --- a/test/gstreamer/gstreamer_test.cpp\n> +++ b/test/gstreamer/gstreamer_test.cpp\n> @@ -78,18 +78,17 @@ GstreamerTest::~GstreamerTest()\n> \n> int GstreamerTest::createPipeline()\n> {\n> -\tg_autoptr(GstElement) libcameraSrc = gst_element_factory_make(\"libcamerasrc\", \"libcamera\");\n> +\tlibcameraSrc_ = 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> +\tif (!libcameraSrc_ || !pipeline_) {\n> \t\tg_printerr(\"Unable to create create pipeline %p.%p\\n\",\n> -\t\t\t libcameraSrc, pipeline_);\n> +\t\t\t libcameraSrc_, pipeline_);\n> \n> \t\treturn TestFail;\n> \t}\n> \n> -\tlibcameraSrc_ = reinterpret_cast(g_steal_pointer(&libcameraSrc));\n> +\tg_object_ref_sink(libcameraSrc_);\n> \n> \treturn TestPass;\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 F10CBBDB1D\n\tfor ;\n\tTue, 14 Sep 2021 17:43:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 700D269183;\n\tTue, 14 Sep 2021 19:43:42 +0200 (CEST)","from mail-qk1-x731.google.com (mail-qk1-x731.google.com\n\t[IPv6:2607:f8b0:4864:20::731])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E47D60132\n\tfor ;\n\tTue, 14 Sep 2021 19:43:41 +0200 (CEST)","by mail-qk1-x731.google.com with SMTP id t4so223480qkb.9\n\tfor ;\n\tTue, 14 Sep 2021 10:43:41 -0700 (PDT)","from nicolas-tpx395.localdomain (173-246-12-168.qc.cable.ebox.net.\n\t[173.246.12.168]) by smtp.gmail.com with ESMTPSA id\n\ti19sm7757159qka.53.2021.09.14.10.43.39\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 14 Sep 2021 10:43:39 -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=\"GicjYLbm\"; 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=lmbUSg3a4vUGxZYgjCoaMnwES0c/nXAxi+rkdZZH9Rw=;\n\tb=GicjYLbmEStwootimV+vDDeZFaNXlmE1RUhym/1RhmYklViOwPI/qZqMqzNdnkPN2p\n\tcQ4HS77sfwF5UDBx1Hx60Az8IrUS+naWJCTwXpc39RrI9CcTNB4LjBg4NEuGtQno0b/U\n\tyuJIt0vdwOr7UsCAyMmEqYIp5+fiOFxLlgvxL/mhrjmEqBO4oiT0raIwCmNnogybDDff\n\t7vpMXYqts6hSJAymtSWlEtm7S8qlwayEf2ywnlkEAIRBURgLkgirDdX7hz93G+2Dj5pR\n\tb/gTb9sS7kKg2ACkW8iIOKh62DfAZhx7mXPYGcYvnXIuu+hYn19RXuK4Hl5NgfZoYzKQ\n\t689A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\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=lmbUSg3a4vUGxZYgjCoaMnwES0c/nXAxi+rkdZZH9Rw=;\n\tb=rgtr+nm/9qN/HAs57Thos38b99jKTbzoW/359bIgjeChLlX7neo4P9EsOPchsuoSKD\n\t9iSSNjpNZm4srRHmTHYLhfhhHVHtvBxZ4lhpImvggWIB0/If9gzlzroiZBEFKTP2ADL3\n\t96qRGDKuWemYaGpmz/IE+OM8bHhovs/Xdzc8AgYG7hEQ9n0I1z1xBKrCvIy3ekn4QFn7\n\tZW6buGjRZDfwGsJe6C0Kz0SDOfxaKk95zjdF1eh5Kt3prbXBHGdrntXPfeilM02MUiL9\n\tuR9vtfxgnLJ4sN7Fwl12mZr9sgIUTzy4PJF62Qt3i4Fg74CmqbzX9ANnYLfevhZ9PGSy\n\t2lzw==","X-Gm-Message-State":"AOAM531a94rGLj7MiADYUqsM9KlKDxUAXLrW7DJNEaw94u+HEvBe/SJX\n\tWwId9eOWuhrqUZPpclvb9kUgFgxyBHPm3Q==","X-Google-Smtp-Source":"ABdhPJylvZuw4EF7l2UTIKQBdT/OX8G8B7agkYgxcL8k1lkjXNiXgH50VtnqJRgwZGzUj7O1V9guQg==","X-Received":"by 2002:a37:e06:: with SMTP id 6mr6241855qko.290.1631641419840; \n\tTue, 14 Sep 2021 10:43:39 -0700 (PDT)","Message-ID":"","From":"Nicolas Dufresne ","To":"Umang Jain , \n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 14 Sep 2021 13:43:38 -0400","In-Reply-To":"<20210914121700.122591-2-umang.jain@ideasonboard.com>","References":"<20210914121700.122591-1-umang.jain@ideasonboard.com>\n\t<20210914121700.122591-2-umang.jain@ideasonboard.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 1/2] test: gstreamer: Simplify\n\telements' ownerships","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\" "}}]