[{"id":5085,"web_url":"https://patchwork.libcamera.org/comment/5085/","msgid":"<20200606091345.GS5864@oden.dyn.berto.se>","date":"2020-06-06T09:13:45","subject":"Re: [libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Support MJPEG","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work.\n\nOn 2020-06-06 17:07:55 +0900, Paul Elder wrote:\n> Add an entry for MJPEG in V4L2CameraProxy's PixelFormatInfo list to\n> allow proper calculation of sizeimage for MJPEG, such that the\n> parameters to mmap can align properly instead of failing. This allows\n> MJPEG to be used in the V4L2 compatibility layer.\n> \n> Signed-off-by: Paul Elder \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/v4l2/v4l2_camera_proxy.cpp | 8 +++++---\n> 1 file changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 059f3cbe..e4511207 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -358,8 +358,8 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)\n> \t * don't support streaming mmap. Since we don't support readwrite and\n> \t * userptr either, the application will get confused and think that\n> \t * we don't support anything.\n> -\t * On the other hand, if a format has a zero sizeimage (eg. MJPEG),\n> -\t * we'll get a floating point exception when we try to stream it.\n> +\t * On the other hand, if a format has a zero sizeimage we'll get a\n> +\t * floating point exception when we try to stream it.\n> \t */\n> \tif (sizeimage_ == 0)\n> \t\tLOG(V4L2Compat, Warning)\n> @@ -556,7 +556,7 @@ struct PixelFormatInfo {\n> \n> namespace {\n> \n> -static const std::array pixelFormatInfo = {{\n> +static const std::array pixelFormatInfo = {{\n> \t/* RGB formats. */\n> \t{ PixelFormat(DRM_FORMAT_RGB888),\tV4L2_PIX_FMT_BGR24,\t1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> \t{ PixelFormat(DRM_FORMAT_BGR888),\tV4L2_PIX_FMT_RGB24,\t1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> @@ -573,6 +573,8 @@ static const std::array pixelFormatInfo = {{\n> \t{ PixelFormat(DRM_FORMAT_NV61),\t\tV4L2_PIX_FMT_NV61,\t2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },\n> \t{ PixelFormat(DRM_FORMAT_NV24),\t\tV4L2_PIX_FMT_NV24,\t2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },\n> \t{ PixelFormat(DRM_FORMAT_NV42),\t\tV4L2_PIX_FMT_NV42,\t2, {{ { 8, 1, 1 }, { 16, 1, 1 }, { 0, 0, 0 } }} },\n> +\t/* Compressed formats. */\n> +\t{ PixelFormat(DRM_FORMAT_MJPEG),\tV4L2_PIX_FMT_MJPEG,\t1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> }};\n> \n> } /* namespace */\n> -- \n> 2.20.1\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-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEFEA600F7\n\tfor ;\n\tSat, 6 Jun 2020 11:13:47 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id w15so7196584lfe.11\n\tfor ;\n\tSat, 06 Jun 2020 02:13:47 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tm13sm1708298ljj.20.2020.06.06.02.13.45\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 06 Jun 2020 02:13:46 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"Xa/Pfgbj\"; \n\tdkim-atps=neutral","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=NUTQ1J22PYIv3jlqI0wxRrOJrmGPVCLVWMUJjGnhkbM=;\n\tb=Xa/PfgbjNE+BRx1yas8MeDutsZiEMQG02kl10W0VajjnLSJyZZD3fZDgmxC4zU282H\n\tqgZ8E+0gwgonSBx7qWL0/h2EAUrI7G4VnRm0VEg64cc7aboPBY7/vjmV0O1VkcUeh/+G\n\tiWJhKA73lq2CiwOhMlbo0uiIFhRI2+5Afze8tnJCbgv/gbfLbra4+WTkF6ZjVCof8cOG\n\t7Yov1xX2b6L8fS5lfK17eG6FRxf/1QhVjEOBT8F6DjU1od406Q2MZ2XVb7HNk+K1gbKG\n\tosnrAGpzYjEguhuCBpFg/Vx1k7w5Gy3vYErIbGolVA5aB6LuX/lEu06Z/jmOhoR16L7J\n\trCVg==","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=NUTQ1J22PYIv3jlqI0wxRrOJrmGPVCLVWMUJjGnhkbM=;\n\tb=uh8+J/Me3g7yxqzqPPy3jMTv/StPctvyC7mq87pveHOwfttlJXyKY/vf9aKjVgArnu\n\t/mk7IOEVtAu/u7JdkqQA4UrviUTuL8i47sdq8W5Am0smoGiYbo9fdZMl3Tsugexsr+Sw\n\tWU4zHhHV8rk0IPkUi8XIOwJyxkgRbPXyNHclhjoRuSulRVeYsI0FEKolWYd91lC2o0+e\n\tFipftBZl1iVX5E6sYaYqn/I041o0jQSszGnKtOaGRQIB3ZE3te11/nTOXMwyugPyOtfk\n\t9A5D6GlvyBhXjxjWxJRJl+kaUVMRBd0BMLGRyv4MOwnlNxhPoi1bCAdNBKEx41jWIngS\n\tA3hA==","X-Gm-Message-State":"AOAM531z1q38+ARg5oRiJKly6qrTqbo4JIsXmdgRW3PYUMcPtY1LOtHp\n\tQQys+fUu5BwAvctO23ycCzwkNiWOLs4=","X-Google-Smtp-Source":"ABdhPJyGy5P7B/RwLhNpEQB4dMj/8EJRCjHP2DYButCYHAgSi+9anj2TsxMhqLfBzOFXbvueA/FMQQ==","X-Received":"by 2002:a19:b06:: with SMTP id 6mr7498540lfl.104.1591434827053; \n\tSat, 06 Jun 2020 02:13:47 -0700 (PDT)","Date":"Sat, 6 Jun 2020 11:13:45 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200606091345.GS5864@oden.dyn.berto.se>","References":"<20200606080755.14408-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200606080755.14408-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Support MJPEG","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":"Sat, 06 Jun 2020 09:13:48 -0000"}},{"id":5088,"web_url":"https://patchwork.libcamera.org/comment/5088/","msgid":"<20200606110324.GB6123@pendragon.ideasonboard.com>","date":"2020-06-06T11:03:24","subject":"Re: [libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Support MJPEG","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sat, Jun 06, 2020 at 05:07:55PM +0900, Paul Elder wrote:\n> Add an entry for MJPEG in V4L2CameraProxy's PixelFormatInfo list to\n> allow proper calculation of sizeimage for MJPEG, such that the\n> parameters to mmap can align properly instead of failing. This allows\n> MJPEG to be used in the V4L2 compatibility layer.\n> \n> Signed-off-by: Paul Elder \n\nThis works, but it's quite a bit of a hack. It will result in an\noverestimate of the required buffer size, while devices that can provide\nMJPEG natively (that's just UVC really) have the ability to provide a\nbetter estimate. The uvcvideo driver reports the estimate through the\nsizeimage field in the V4L2 format ioctls.\n\nYou're not the only one to take the short route though, there are UVC\ndevices that report the same value as this patch :-)\n\nWe have a field in StreamConfiguration to report the stride, but no\nfield to report the full image size. Would it make sense to add one ? Or\nare there better options ?\n\nI think we can merge this patch for now, but I wouldn't stop here. Could\nyou add a todo comment to capture this ?\n\n> ---\n> src/v4l2/v4l2_camera_proxy.cpp | 8 +++++---\n> 1 file changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 059f3cbe..e4511207 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -358,8 +358,8 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)\n> \t * don't support streaming mmap. Since we don't support readwrite and\n> \t * userptr either, the application will get confused and think that\n> \t * we don't support anything.\n> -\t * On the other hand, if a format has a zero sizeimage (eg. MJPEG),\n> -\t * we'll get a floating point exception when we try to stream it.\n> +\t * On the other hand, if a format has a zero sizeimage we'll get a\n> +\t * floating point exception when we try to stream it.\n> \t */\n> \tif (sizeimage_ == 0)\n> \t\tLOG(V4L2Compat, Warning)\n\nShould this still be a warning, or should it be turned back into an\nerror ? I think the comment should then be updated accordingly.\n\n> @@ -556,7 +556,7 @@ struct PixelFormatInfo {\n> \n> namespace {\n> \n> -static const std::array pixelFormatInfo = {{\n> +static const std::array pixelFormatInfo = {{\n> \t/* RGB formats. */\n> \t{ PixelFormat(DRM_FORMAT_RGB888),\tV4L2_PIX_FMT_BGR24,\t1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> \t{ PixelFormat(DRM_FORMAT_BGR888),\tV4L2_PIX_FMT_RGB24,\t1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> @@ -573,6 +573,8 @@ static const std::array pixelFormatInfo = {{\n> \t{ PixelFormat(DRM_FORMAT_NV61),\t\tV4L2_PIX_FMT_NV61,\t2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },\n> \t{ PixelFormat(DRM_FORMAT_NV24),\t\tV4L2_PIX_FMT_NV24,\t2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },\n> \t{ PixelFormat(DRM_FORMAT_NV42),\t\tV4L2_PIX_FMT_NV42,\t2, {{ { 8, 1, 1 }, { 16, 1, 1 }, { 0, 0, 0 } }} },\n> +\t/* Compressed formats. */\n> +\t{ PixelFormat(DRM_FORMAT_MJPEG),\tV4L2_PIX_FMT_MJPEG,\t1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> }};\n> \n> } /* namespace */","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 AB65161167\n\tfor ;\n\tSat, 6 Jun 2020 13:03:43 +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 1128627C;\n\tSat, 6 Jun 2020 13:03:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"L2Juez2W\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591441423;\n\tbh=xjT1YrytF6vadpD4VJBrsHqODO9en5W2K/pGyhAnkSM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L2Juez2WgM9gNYc6DVZKyMyFAAmDSbHLMi5DtNrdhZkYaSkSqFG3WFKJVRlkyYev7\n\tgt8v10Al3IEZbc9G86ZH1XAr0QBMScqDLmYmJd0COeipra/rtp01aB9rqCm/Z4jMKI\n\tMRg/Qedpit9IAO/IYMI/dLIj1tv7cHOdo42udgbE=","Date":"Sat, 6 Jun 2020 14:03:24 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200606110324.GB6123@pendragon.ideasonboard.com>","References":"<20200606080755.14408-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200606080755.14408-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Support MJPEG","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":"Sat, 06 Jun 2020 11:03:43 -0000"}},{"id":5120,"web_url":"https://patchwork.libcamera.org/comment/5120/","msgid":"<20200608075757.GA2943@emerald.amanokami.net>","date":"2020-06-08T07:57:57","subject":"Re: [libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Support MJPEG","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the review.\n\nOn Sat, Jun 06, 2020 at 02:03:24PM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Sat, Jun 06, 2020 at 05:07:55PM +0900, Paul Elder wrote:\n> > Add an entry for MJPEG in V4L2CameraProxy's PixelFormatInfo list to\n> > allow proper calculation of sizeimage for MJPEG, such that the\n> > parameters to mmap can align properly instead of failing. This allows\n> > MJPEG to be used in the V4L2 compatibility layer.\n> > \n> > Signed-off-by: Paul Elder \n> \n> This works, but it's quite a bit of a hack. It will result in an\n> overestimate of the required buffer size, while devices that can provide\n> MJPEG natively (that's just UVC really) have the ability to provide a\n> better estimate. The uvcvideo driver reports the estimate through the\n> sizeimage field in the V4L2 format ioctls.\n> \n> You're not the only one to take the short route though, there are UVC\n> devices that report the same value as this patch :-)\n> \n> We have a field in StreamConfiguration to report the stride, but no\n> field to report the full image size. Would it make sense to add one ? Or\n> are there better options ?\n\nI think we could add one. Or, does stride * height work?\n\n> I think we can merge this patch for now, but I wouldn't stop here. Could\n> you add a todo comment to capture this ?\n\nOkay.\n\n> > ---\n> > src/v4l2/v4l2_camera_proxy.cpp | 8 +++++---\n> > 1 file changed, 5 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > index 059f3cbe..e4511207 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -358,8 +358,8 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)\n> > \t * don't support streaming mmap. Since we don't support readwrite and\n> > \t * userptr either, the application will get confused and think that\n> > \t * we don't support anything.\n> > -\t * On the other hand, if a format has a zero sizeimage (eg. MJPEG),\n> > -\t * we'll get a floating point exception when we try to stream it.\n> > +\t * On the other hand, if a format has a zero sizeimage we'll get a\n> > +\t * floating point exception when we try to stream it.\n> > \t */\n> > \tif (sizeimage_ == 0)\n> > \t\tLOG(V4L2Compat, Warning)\n> \n> Should this still be a warning, or should it be turned back into an\n> error ? I think the comment should then be updated accordingly.\n\nI think warning should be fine. It's a warning, that we can continue,\nbut there might be a problem if you choose a format with zero sizeimage.\nOn the other hand, this warning will only print if that format is set at\nthe time of reqbufs, so it might not display.\n\n> > @@ -556,7 +556,7 @@ struct PixelFormatInfo {\n> > \n> > namespace {\n> > \n> > -static const std::array pixelFormatInfo = {{\n> > +static const std::array pixelFormatInfo = {{\n> > \t/* RGB formats. */\n> > \t{ PixelFormat(DRM_FORMAT_RGB888),\tV4L2_PIX_FMT_BGR24,\t1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> > \t{ PixelFormat(DRM_FORMAT_BGR888),\tV4L2_PIX_FMT_RGB24,\t1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> > @@ -573,6 +573,8 @@ static const std::array pixelFormatInfo = {{\n> > \t{ PixelFormat(DRM_FORMAT_NV61),\t\tV4L2_PIX_FMT_NV61,\t2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },\n> > \t{ PixelFormat(DRM_FORMAT_NV24),\t\tV4L2_PIX_FMT_NV24,\t2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },\n> > \t{ PixelFormat(DRM_FORMAT_NV42),\t\tV4L2_PIX_FMT_NV42,\t2, {{ { 8, 1, 1 }, { 16, 1, 1 }, { 0, 0, 0 } }} },\n> > +\t/* Compressed formats. */\n> > +\t{ PixelFormat(DRM_FORMAT_MJPEG),\tV4L2_PIX_FMT_MJPEG,\t1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> > }};\n> > \n> > } /* namespace */\n\n\nThanks,\n\nPaul","headers":{"Return-Path":"","Received":["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 8D120603C7\n\tfor ;\n\tMon, 8 Jun 2020 09:58:04 +0200 (CEST)","from emerald.amanokami.net (fs76eef344.knge213.ap.nuro.jp\n\t[118.238.243.68])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 551C72C9;\n\tMon, 8 Jun 2020 09:58:03 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"L9+fX3OO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591603084;\n\tbh=H4ImJRJ61s9scEdbLOsOaKIof9NpHZ1H5Bf2KV8ht5o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L9+fX3OOYiyK38ZcHYFikLzwBvC2mmCatS6QR0bkyHqeFEmZnoCh9jV+WvAcLg0Vl\n\thTe/MVZ8RRvmUBJNoQa3JoG5XXgU9tEJnOkOGhvbZpgQz29d7TETX9WdwVNr4GD2k4\n\tPEiY/dFzenqrddV/d/lzqfnw4crVcVVs77PcxY+Q=","Date":"Mon, 8 Jun 2020 16:57:57 +0900","From":"Paul Elder ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200608075757.GA2943@emerald.amanokami.net>","References":"<20200606080755.14408-1-paul.elder@ideasonboard.com>\n\t<20200606110324.GB6123@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20200606110324.GB6123@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Support MJPEG","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, 08 Jun 2020 07:58:04 -0000"}},{"id":5130,"web_url":"https://patchwork.libcamera.org/comment/5130/","msgid":"<20200608131540.GD5896@pendragon.ideasonboard.com>","date":"2020-06-08T13:15:40","subject":"Re: [libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Support MJPEG","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Mon, Jun 08, 2020 at 04:57:57PM +0900, Paul Elder wrote:\n> On Sat, Jun 06, 2020 at 02:03:24PM +0300, Laurent Pinchart wrote:\n> > On Sat, Jun 06, 2020 at 05:07:55PM +0900, Paul Elder wrote:\n> > > Add an entry for MJPEG in V4L2CameraProxy's PixelFormatInfo list to\n> > > allow proper calculation of sizeimage for MJPEG, such that the\n> > > parameters to mmap can align properly instead of failing. This allows\n> > > MJPEG to be used in the V4L2 compatibility layer.\n> > > \n> > > Signed-off-by: Paul Elder \n> > \n> > This works, but it's quite a bit of a hack. It will result in an\n> > overestimate of the required buffer size, while devices that can provide\n> > MJPEG natively (that's just UVC really) have the ability to provide a\n> > better estimate. The uvcvideo driver reports the estimate through the\n> > sizeimage field in the V4L2 format ioctls.\n> > \n> > You're not the only one to take the short route though, there are UVC\n> > devices that report the same value as this patch :-)\n> > \n> > We have a field in StreamConfiguration to report the stride, but no\n> > field to report the full image size. Would it make sense to add one ? Or\n> > are there better options ?\n> \n> I think we could add one. Or, does stride * height work?\n\nstride is ill-defined for compressed formats. I think that regardless of\nwhat we decide to do, we need to think about how all the fields should\nbe used with the different types of formats (packed, planar, compressed,\n...). Could you please make sure to take that into consideration, and\ndocument it in any patch you may submit ?\n\n> > I think we can merge this patch for now, but I wouldn't stop here. Could\n> > you add a todo comment to capture this ?\n> \n> Okay.\n> \n> > > ---\n> > > src/v4l2/v4l2_camera_proxy.cpp | 8 +++++---\n> > > 1 file changed, 5 insertions(+), 3 deletions(-)\n> > > \n> > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > index 059f3cbe..e4511207 100644\n> > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > @@ -358,8 +358,8 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)\n> > > \t * don't support streaming mmap. Since we don't support readwrite and\n> > > \t * userptr either, the application will get confused and think that\n> > > \t * we don't support anything.\n> > > -\t * On the other hand, if a format has a zero sizeimage (eg. MJPEG),\n> > > -\t * we'll get a floating point exception when we try to stream it.\n> > > +\t * On the other hand, if a format has a zero sizeimage we'll get a\n> > > +\t * floating point exception when we try to stream it.\n> > > \t */\n> > > \tif (sizeimage_ == 0)\n> > > \t\tLOG(V4L2Compat, Warning)\n> > \n> > Should this still be a warning, or should it be turned back into an\n> > error ? I think the comment should then be updated accordingly.\n> \n> I think warning should be fine. It's a warning, that we can continue,\n> but there might be a problem if you choose a format with zero sizeimage.\n> On the other hand, this warning will only print if that format is set at\n> the time of reqbufs, so it might not display.\n\nBut is it possible to do so anymore, given that all formats supported by\nlibcamera should now be listed in pixelFormatInfo ?\n\n> > > @@ -556,7 +556,7 @@ struct PixelFormatInfo {\n> > > \n> > > namespace {\n> > > \n> > > -static const std::array pixelFormatInfo = {{\n> > > +static const std::array pixelFormatInfo = {{\n> > > \t/* RGB formats. */\n> > > \t{ PixelFormat(DRM_FORMAT_RGB888),\tV4L2_PIX_FMT_BGR24,\t1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> > > \t{ PixelFormat(DRM_FORMAT_BGR888),\tV4L2_PIX_FMT_RGB24,\t1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> > > @@ -573,6 +573,8 @@ static const std::array pixelFormatInfo = {{\n> > > \t{ PixelFormat(DRM_FORMAT_NV61),\t\tV4L2_PIX_FMT_NV61,\t2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },\n> > > \t{ PixelFormat(DRM_FORMAT_NV24),\t\tV4L2_PIX_FMT_NV24,\t2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },\n> > > \t{ PixelFormat(DRM_FORMAT_NV42),\t\tV4L2_PIX_FMT_NV42,\t2, {{ { 8, 1, 1 }, { 16, 1, 1 }, { 0, 0, 0 } }} },\n> > > +\t/* Compressed formats. */\n> > > +\t{ PixelFormat(DRM_FORMAT_MJPEG),\tV4L2_PIX_FMT_MJPEG,\t1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },\n> > > }};\n> > > \n> > > } /* namespace */","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 21DFF61027\n\tfor ;\n\tMon, 8 Jun 2020 15:16:00 +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 82D3B2C9;\n\tMon, 8 Jun 2020 15:15:59 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"T6JD7rFx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591622159;\n\tbh=jWzYLkPbmRPKJZULzsLcDU0dBich0JzOGjPgBRiKxhk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=T6JD7rFxkJxcQa9LrRMEef2RX2TUoeQh+BUFCobwknRwavc+G2J/LN/EKPbZ/Nvkg\n\t61CqRKSxOBOTxmCKngAQKMi2xWNuREoq7WEVuDnTSm//WmQe5ofYhy3IyX8ixWd23a\n\tayBkMK5jg/BTbR7DXLLerJ8JrqSqzZAWcwNJKGEo=","Date":"Mon, 8 Jun 2020 16:15:40 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200608131540.GD5896@pendragon.ideasonboard.com>","References":"<20200606080755.14408-1-paul.elder@ideasonboard.com>\n\t<20200606110324.GB6123@pendragon.ideasonboard.com>\n\t<20200608075757.GA2943@emerald.amanokami.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200608075757.GA2943@emerald.amanokami.net>","Subject":"Re: [libcamera-devel] [PATCH] v4l2: v4l2_camera_proxy: Support MJPEG","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, 08 Jun 2020 13:16:00 -0000"}}]