[{"id":4027,"web_url":"https://patchwork.libcamera.org/comment/4027/","msgid":"<20200316134931.GD2260535@oden.dyn.berto.se>","date":"2020-03-16T13:49:31","subject":"Re: [libcamera-devel] [PATCH 4/9] libcamera: v4l2_videodevice:\n\tRefactor allocateBuffers()","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 patch.\n\nOn 2020-03-15 01:57:23 +0200, Laurent Pinchart wrote:\n> Move the buffer creation code out of allocateBuffers() to a\n> createBuffers() function. This prepare for the rework of buffer export\n> and will avoid code duplication.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/include/v4l2_videodevice.h | 4 +-\n> src/libcamera/v4l2_videodevice.cpp | 68 +++++++++++++-----------\n> 2 files changed, 39 insertions(+), 33 deletions(-)\n> \n> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h\n> index 26f5e5917716..358d89e57414 100644\n> --- a/src/libcamera/include/v4l2_videodevice.h\n> +++ b/src/libcamera/include/v4l2_videodevice.h\n> @@ -228,7 +228,9 @@ private:\n> \tint setSelection(unsigned int target, Rectangle *rect);\n> \n> \tint requestBuffers(unsigned int count, enum v4l2_memory memoryType);\n> -\tstd::unique_ptr createBuffer(const struct v4l2_buffer &buf);\n> +\tint createBuffers(unsigned int count,\n> +\t\t\t std::vector> *buffers);\n> +\tstd::unique_ptr createBuffer(unsigned int index);\n> \tFileDescriptor exportDmabufFd(unsigned int index, unsigned int plane);\n> \n> \tvoid bufferAvailable(EventNotifier *notifier);\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 6911ab024fd7..d02b02ef77d6 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -1052,61 +1052,65 @@ int V4L2VideoDevice::requestBuffers(unsigned int count,\n> */\n> int V4L2VideoDevice::allocateBuffers(unsigned int count,\n> \t\t\t\t std::vector> *buffers)\n> +{\n> +\tint ret = createBuffers(count, buffers);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tcache_ = new V4L2BufferCache(*buffers);\n> +\tmemoryType_ = V4L2_MEMORY_MMAP;\n> +\n> +\treturn ret;\n> +}\n> +\n> +int V4L2VideoDevice::createBuffers(unsigned int count,\n> +\t\t\t\t std::vector> *buffers)\n> {\n> \tif (cache_) {\n> \t\tLOG(V4L2, Error) << \"Buffers already allocated\";\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tmemoryType_ = V4L2_MEMORY_MMAP;\n> -\n> \tint ret = requestBuffers(count, V4L2_MEMORY_MMAP);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> \tfor (unsigned i = 0; i < count; ++i) {\n> -\t\tstruct v4l2_buffer buf = {};\n> -\t\tstruct v4l2_plane planes[VIDEO_MAX_PLANES] = {};\n> -\n> -\t\tbuf.index = i;\n> -\t\tbuf.type = bufferType_;\n> -\t\tbuf.memory = memoryType_;\n> -\t\tbuf.length = ARRAY_SIZE(planes);\n> -\t\tbuf.m.planes = planes;\n> -\n> -\t\tret = ioctl(VIDIOC_QUERYBUF, &buf);\n> -\t\tif (ret < 0) {\n> -\t\t\tLOG(V4L2, Error)\n> -\t\t\t\t<< \"Unable to query buffer \" << i << \": \"\n> -\t\t\t\t<< strerror(-ret);\n> -\t\t\tgoto err_buf;\n> -\t\t}\n> -\n> -\t\tstd::unique_ptr buffer = createBuffer(buf);\n> +\t\tstd::unique_ptr buffer = createBuffer(i);\n> \t\tif (!buffer) {\n> \t\t\tLOG(V4L2, Error) << \"Unable to create buffer\";\n> -\t\t\tret = -EINVAL;\n> -\t\t\tgoto err_buf;\n> +\n> +\t\t\trequestBuffers(0, V4L2_MEMORY_MMAP);\n> +\t\t\tbuffers->clear();\n> +\n> +\t\t\treturn -EINVAL;\n> \t\t}\n> \n> \t\tbuffers->push_back(std::move(buffer));\n> \t}\n> \n> -\tcache_ = new V4L2BufferCache(*buffers);\n> -\n> \treturn count;\n> +}\n> \n> -err_buf:\n> -\trequestBuffers(0, V4L2_MEMORY_MMAP);\n> +std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index)\n> +{\n> +\tstruct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {};\n> +\tstruct v4l2_buffer buf = {};\n> \n> -\tbuffers->clear();\n> +\tbuf.index = index;\n> +\tbuf.type = bufferType_;\n> +\tbuf.memory = V4L2_MEMORY_MMAP;\n> +\tbuf.length = ARRAY_SIZE(v4l2Planes);\n> +\tbuf.m.planes = v4l2Planes;\n> \n> -\treturn ret;\n> -}\n> +\tint ret = ioctl(VIDIOC_QUERYBUF, &buf);\n> +\tif (ret < 0) {\n> +\t\tLOG(V4L2, Error)\n> +\t\t\t<< \"Unable to query buffer \" << index << \": \"\n> +\t\t\t<< strerror(-ret);\n> +\t\treturn nullptr;\n> +\t}\n> \n> -std::unique_ptr\n> -V4L2VideoDevice::createBuffer(const struct v4l2_buffer &buf)\n> -{\n> \tconst bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);\n> \tconst unsigned int numPlanes = multiPlanar ? buf.length : 1;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7C65B6041A\n\tfor ;\n\tMon, 16 Mar 2020 14:49:33 +0100 (CET)","by mail-lf1-x144.google.com with SMTP id a28so2271466lfr.13\n\tfor ;\n\tMon, 16 Mar 2020 06:49:33 -0700 (PDT)","from localhost (h-200-138.A463.priv.bahnhof.se. [176.10.200.138])\n\tby smtp.gmail.com with ESMTPSA id\n\tk21sm10071110lfj.22.2020.03.16.06.49.31\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 16 Mar 2020 06:49:32 -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\tbh=dZ2XzZACnsmUXKAJ3Co8gtZsXsA8ufSu1ml1VHXrJtA=;\n\tb=KviFqMuHUu+e4pAojqksAqt3W1r5rPGc0IofSM8X6wXkIOIVyX1W8OAkljPEUE1eYc\n\tDfhlmVMxoh9KCta5lgiYMMVp0Lqrljl7ZG5wLemAXCL39IIIyZFzcmqv3XGmwGdZKIbv\n\tQTmuPwXKxZXXpEwtSeRr0IRp7oYS3gamiB4GdLjSDuZvIWnIu2DI73sz7MgolUiiIUx4\n\ttLq+RGCts5rTUPasbC0XH5JMijadLBrw/NmqSX7kH5l9t0BqMtlPVa60Nnoc2Rcmxvmf\n\tlBuquyfqjyJxYDxdn5uTBgLM06O1C47HNwf22NLgq7/5nZL3F7ofZDph8IdtTSLnnfkk\n\tt/Gw==","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;\n\tbh=dZ2XzZACnsmUXKAJ3Co8gtZsXsA8ufSu1ml1VHXrJtA=;\n\tb=YKLtZPPiDMjabRMhyS9Iudssis3rMF7LiE69yX/V9i3yXjduLDgjDnbZxq1CinLOfi\n\tSXR6Pdzf5KI374Zhnca+CzrSgQusiqfYgZ1YE2ZS6419ygJZW3se3CGEKG0mSuacBjdA\n\t2t5BmWrtYlfY8WwHmlWuWfst+KM6EEKYcWnVq4tbe+h+TgjWXkgN8k0xFJf9XMfJ3P5H\n\t7zQnSBGGwOWmzrq/QfBh1EZWhzf6Kev04UoHq+6nGhjWQN3e2l6GexoTxrbb9eBhMzJ4\n\tNlGfmFoBcnYOYXgJYBSTcSvGe/dMYsuwJO5ySFH2QbkEUnCohNdW0aE0MI2r9d5FwOR/\n\tXlYQ==","X-Gm-Message-State":"ANhLgQ2qzdSOAXF6oMICzKaVvAVgZKmGeGu1iv0Oqryi9/4sjFga1yNn\n\tw4UBM28VnwhZmw9vqTNs635Pew==","X-Google-Smtp-Source":"ADFU+vtfEgdQm4Eiu0iSqSg4bll5rsFU4DSqxvzmOrzJwifpyauuKsmLEaMhZJf6WuQ90vWGab//Yg==","X-Received":"by 2002:a19:ae0b:: with SMTP id\n\tf11mr7992885lfc.190.1584366572716; \n\tMon, 16 Mar 2020 06:49:32 -0700 (PDT)","Date":"Mon, 16 Mar 2020 14:49:31 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200316134931.GD2260535@oden.dyn.berto.se>","References":"<20200314235728.15495-1-laurent.pinchart@ideasonboard.com>\n\t<20200314235728.15495-5-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200314235728.15495-5-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 4/9] libcamera: v4l2_videodevice:\n\tRefactor allocateBuffers()","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","X-List-Received-Date":"Mon, 16 Mar 2020 13:49:33 -0000"}}]