[{"id":1421,"web_url":"https://patchwork.libcamera.org/comment/1421/","msgid":"<20190416231454.GQ4822@pendragon.ideasonboard.com>","date":"2019-04-16T23:14:54","subject":"Re: [libcamera-devel] [RFC 05/11] libcamera: media_device: Make\n\topen() and close() private","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Sun, Apr 14, 2019 at 03:35:00AM +0200, Niklas Söderlund wrote:\n> All external callers to open() and close() have been refactored and\n> there is no need to expose these functions anymore, make them private.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/include/media_device.h | 6 +-\n> src/libcamera/media_device.cpp | 78 +++++++------------\n> test/media_device/media_device_print_test.cpp | 11 ---\n> 3 files changed, 32 insertions(+), 63 deletions(-)\n> \n> diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h\n> index 883985055eb419dd..e513a2fee1b91a1b 100644\n> --- a/src/libcamera/include/media_device.h\n> +++ b/src/libcamera/include/media_device.h\n> @@ -30,9 +30,6 @@ public:\n> \tvoid release();\n> \tbool busy() const { return acquired_; }\n> \n> -\tint open();\n> -\tvoid close();\n> -\n> \tint populate();\n> \tbool valid() const { return valid_; }\n> \n> @@ -62,6 +59,9 @@ private:\n> \tbool valid_;\n> \tbool acquired_;\n> \n> +\tint open();\n> +\tvoid close();\n> +\n\nWe usually declare functions before variables.\n\n> \tstd::map objects_;\n> \tMediaObject *object(unsigned int id);\n> \tbool addObject(MediaObject *object);\n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index 644c6143fb15e1fa..0138be526f886c90 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -124,55 +124,6 @@ void MediaDevice::release()\n> * \\sa acquire(), release()\n> */\n> \n> -/**\n> - * \\brief Open a media device and retrieve device information\n> - *\n> - * If the device is already open the function returns -EBUSY.\n> - *\n> - * \\return 0 on success or a negative error code otherwise\n> - */\n> -int MediaDevice::open()\n> -{\n> -\tif (fd_ != -1) {\n> -\t\tLOG(MediaDevice, Error) << \"MediaDevice already open\";\n> -\t\treturn -EBUSY;\n> -\t}\n> -\n> -\tint ret = ::open(deviceNode_.c_str(), O_RDWR);\n> -\tif (ret < 0) {\n> -\t\tret = -errno;\n> -\t\tLOG(MediaDevice, Error)\n> -\t\t\t<< \"Failed to open media device at \"\n> -\t\t\t<< deviceNode_ << \": \" << strerror(-ret);\n> -\t\treturn ret;\n> -\t}\n> -\tfd_ = ret;\n> -\n> -\treturn 0;\n> -}\n> -\n> -/**\n> - * \\brief Close the media device\n> - *\n> - * This function closes the media device node. It does not invalidate the media\n> - * graph and all cached media objects remain valid and can be accessed normally.\n> - * Once closed no operation interacting with the media device node can be\n> - * performed until the device is opened again.\n> - *\n> - * Closing an already closed device is allowed and will not perform any\n> - * operation.\n> - *\n> - * \\sa open()\n> - */\n> -void MediaDevice::close()\n> -{\n> -\tif (fd_ == -1)\n> -\t\treturn;\n> -\n> -\t::close(fd_);\n> -\tfd_ = -1;\n> -}\n> -\n> /**\n> * \\brief Populate the media graph with media objects\n> *\n> @@ -440,6 +391,35 @@ int MediaDevice::disableLinks()\n> * driver unloading for most devices. The media device is passed as a parameter.\n> */\n> \n\nI would keep the documentation, it's still valuable.\n\n> +int MediaDevice::open()\n> +{\n> +\tif (fd_ != -1) {\n> +\t\tLOG(MediaDevice, Error) << \"MediaDevice already open\";\n> +\t\treturn -EBUSY;\n> +\t}\n> +\n> +\tint ret = ::open(deviceNode_.c_str(), O_RDWR);\n> +\tif (ret < 0) {\n> +\t\tret = -errno;\n> +\t\tLOG(MediaDevice, Error)\n> +\t\t\t<< \"Failed to open media device at \"\n> +\t\t\t<< deviceNode_ << \": \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\tfd_ = ret;\n> +\n> +\treturn 0;\n> +}\n> +\n> +void MediaDevice::close()\n> +{\n> +\tif (fd_ == -1)\n> +\t\treturn;\n> +\n> +\t::close(fd_);\n> +\tfd_ = -1;\n> +}\n> +\n> /**\n> * \\var MediaDevice::objects_\n> * \\brief Global map of media objects (entities, pads, links) keyed by their\n> diff --git a/test/media_device/media_device_print_test.cpp b/test/media_device/media_device_print_test.cpp\n> index ceffd538e13fca73..30d929b8c76387a7 100644\n> --- a/test/media_device/media_device_print_test.cpp\n> +++ b/test/media_device/media_device_print_test.cpp\n> @@ -113,17 +113,6 @@ int MediaDevicePrintTest::testMediaDevice(const string deviceNode)\n> \tMediaDevice dev(deviceNode);\n> \tint ret;\n> \n> -\t/* Fuzzy open/close sequence. */\n\nDo we need a fuzzy acquire/release sequence ?\n\n> -\tret = dev.open();\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tret = dev.open();\n> -\tif (!ret)\n> -\t\treturn ret;\n> -\n> -\tdev.close();\n> -\n> \tret = dev.populate();\n> \tif (ret)\n> \t\treturn ret;","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5BAB960004\n\tfor ;\n\tWed, 17 Apr 2019 01:15:04 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C336CE2;\n\tWed, 17 Apr 2019 01:15:03 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1555456504;\n\tbh=G/T7wJVuI/uffy7087z4zrNmC9u8ET49r9qou6RS5w4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=B5nnzUuHChGGHcEiHfvxBi1KIzdk9Tv7OY5/eE4cioRtUv69Q/PgP0uBGxLM1q4+Y\n\tjHxRVzcmwYEg03d8L3DlkREN3F2prsJF8A/sAFPgG0+mAKW7a+2GM8/rzjWayZNtX1\n\tz+v1IHcRywSKOOYpPC1lzAIW149cSnq9SIl5VZrQ=","Date":"Wed, 17 Apr 2019 02:14:54 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190416231454.GQ4822@pendragon.ideasonboard.com>","References":"<20190414013506.10515-1-niklas.soderlund@ragnatech.se>\n\t<20190414013506.10515-6-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190414013506.10515-6-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC 05/11] libcamera: media_device: Make\n\topen() and close() private","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 16 Apr 2019 23:15:04 -0000"}},{"id":1535,"web_url":"https://patchwork.libcamera.org/comment/1535/","msgid":"<20190429180507.GK16573@bigcity.dyn.berto.se>","date":"2019-04-29T18:05:08","subject":"Re: [libcamera-devel] [RFC 05/11] libcamera: media_device: Make\n\topen() and close() private","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-04-17 02:14:54 +0300, Laurent Pinchart wrote:\n\nsnip\n\n> > diff --git a/test/media_device/media_device_print_test.cpp \n> > b/test/media_device/media_device_print_test.cpp\n> > index ceffd538e13fca73..30d929b8c76387a7 100644\n> > --- a/test/media_device/media_device_print_test.cpp\n> > +++ b/test/media_device/media_device_print_test.cpp\n> > @@ -113,17 +113,6 @@ int MediaDevicePrintTest::testMediaDevice(const string deviceNode)\n> > \tMediaDevice dev(deviceNode);\n> > \tint ret;\n> > \n> > -\t/* Fuzzy open/close sequence. */\n> \n> Do we need a fuzzy acquire/release sequence ?\n\nThat is a good idea, I will add this for v1 to replace this test.\n\n> \n> > -\tret = dev.open();\n> > -\tif (ret)\n> > -\t\treturn ret;\n> > -\n> > -\tret = dev.open();\n> > -\tif (!ret)\n> > -\t\treturn ret;\n> > -\n> > -\tdev.close();\n> > -\n> > \tret = dev.populate();\n> > \tif (ret)\n> > \t\treturn ret;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com\n\t[IPv6:2a00:1450:4864:20::12c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D61B600EC\n\tfor ;\n\tMon, 29 Apr 2019 20:05:10 +0200 (CEST)","by mail-lf1-x12c.google.com with SMTP id j20so8758213lfh.2\n\tfor ;\n\tMon, 29 Apr 2019 11:05:10 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tq125sm2883470ljb.76.2019.04.29.11.05.08\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 29 Apr 2019 11:05:08 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=Lj0hgEDcIY22TQgSL+I7uPjODRp+WrXvfopJ7Uo4DLw=;\n\tb=asdZ1rFq5HRfFF4I+j6DmQnMqfVFAvt+rd3n6GaGtoqaxd82k/2A+sMn720NKpMI5n\n\tF0TwsrZ3ORu9ijrO50wIuHFYx9rU3hII/82Fk8H3OmCSPS3CgkIPZOzclw7uWZ7O6T2x\n\tCwuIHXLLOlUtX6yCL+t6LH7PuFWUKuHYXNIrGDQtoeJAk8CJaqiZyHb10TYHHEfQRnBG\n\tPUyI0dKnhrRxV2eZg5y0Ayi5ckxNplA0AXfljCBD0JdM5Nwm8EPKQTaLyeVYypKd0wO0\n\t0ou+9S5TSZkW2EDrEHh1xD1KKTOz0KPbs9hsgg3uvGi0huMEgTVSI2O/GDu9lg1AQ4+v\n\tDlUQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=Lj0hgEDcIY22TQgSL+I7uPjODRp+WrXvfopJ7Uo4DLw=;\n\tb=cv/N+UC4CzFFP7FJeh0FQnr9E6FS/dxAgF4vYdOGc455TT601mEyZMYCfXCswZCC8C\n\tnLwEL/UIa6LsoAQWsUyOoZ4Hcd8RaOcJVzc0WhrkcPqdTT9tpmA+hg7ABe/cKSLpKSKx\n\tGg4D3NdujJQkUUL7PgB81hiBaj8hZO1US+SaXdGlbiTlvRyga+DElf+qkVc9yNCnQenj\n\thH/argWu1Zd+xLlV0qNM9r0dj/1zWnxxgKL8Bxnmhr8EavOVAXMgsYlBuhsvqQmPCizm\n\tZzKGUXxc1eTdWYgRe09fG/EohKNTfg37tDtI3yGXxXo++QdQKEt+K35Ct+GT8xQyIX+A\n\tyrXA==","X-Gm-Message-State":"APjAAAVBrHjGx138cutg6ugl5Auk/TQRUTxhG5VV+n9Rs9A4UrOe10/U\n\thYkuO0WLtHVsJPeBHRTm9HJmvw==","X-Google-Smtp-Source":"APXvYqxePX6DhmEcP7hB2gu4YkPy5p+qgfK9rD2GOHHnX3JyxbY+dt5zYyYR4AGnZX2IRyz/0Dj0Xw==","X-Received":"by 2002:a19:7402:: with SMTP id v2mr18362159lfe.53.1556561109125;\n\tMon, 29 Apr 2019 11:05:09 -0700 (PDT)","Date":"Mon, 29 Apr 2019 20:05:08 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190429180507.GK16573@bigcity.dyn.berto.se>","References":"<20190414013506.10515-1-niklas.soderlund@ragnatech.se>\n\t<20190414013506.10515-6-niklas.soderlund@ragnatech.se>\n\t<20190416231454.GQ4822@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190416231454.GQ4822@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [RFC 05/11] libcamera: media_device: Make\n\topen() and close() private","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 29 Apr 2019 18:05:10 -0000"}}]