[{"id":12105,"web_url":"https://patchwork.libcamera.org/comment/12105/","msgid":"","date":"2020-08-24T09:32:24","subject":"Re: [libcamera-devel] [PATCH] cam: Limit file write to payload size","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 22/08/2020 14:40, Laurent Pinchart wrote:\n> The payload size in a captured framebuffer is usually equal to the\n> buffer size. However, for compressed formats, the payload may be\n> smaller Only write the payload when capturing to a file.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/cam/buffer_writer.cpp | 12 ++++++++++--\n> 1 file changed, 10 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp\n> index c5a5eb46224a..6305958924a4 100644\n> --- a/src/cam/buffer_writer.cpp\n> +++ b/src/cam/buffer_writer.cpp\n> @@ -64,9 +64,17 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName)\n> \tif (fd == -1)\n> \t\treturn -errno;\n> \n> -\tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n> +\tfor (unsigned int i = 0; i < buffer->planes().size(); ++i) {\n> +\t\tconst FrameBuffer::Plane &plane = buffer->planes()[i];\n> +\t\tconst FrameMetadata::Plane &meta = buffer->metadata().planes[i];\n> +\n> \t\tvoid *data = mappedBuffers_[plane.fd.fd()].first;\n> -\t\tunsigned int length = plane.length;\n> +\t\tunsigned int length = std::min(meta.bytesused, plane.length);\n> +\n> +\t\tif (length < meta.bytesused)\n> +\t\t\tstd::cerr << \"payload size \" << meta.bytesused\n> +\t\t\t\t << \" smaller than plane size \" << plane.length\n> +\t\t\t\t << std::endl;\n\nIs the cerr necessary? If saving out an MJPEG stream from an MJPEG UVC\ncamera - this would print every frame...\n\nWhich is however the use case that makes me believe this patch is useful...\n\nSo with that considered either way,\n\nReviewed-by: Kieran Bingham \n\n> \n> \t\tret = ::write(fd, data, length);\n> \t\tif (ret < 0) {\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 E62C9BD87C\n\tfor ;\n\tMon, 24 Aug 2020 09:32:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 63C0162818;\n\tMon, 24 Aug 2020 11:32:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B18B960387\n\tfor ;\n\tMon, 24 Aug 2020 11:32:29 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DFFB2279;\n\tMon, 24 Aug 2020 11:32:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ecxYyXsV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598261547;\n\tbh=nVMdwUsV+TNFXBHW1vNfUtUeHJb1St6ZrQIHTuzgk2Q=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=ecxYyXsV613tFMLXGTpM/MXtzXXypKmxo11X/MYj1pW2FGoSn24izUkSmfl63UywU\n\trdzyRbWn/CYhz0xEi7IX578T4OTgdrEE/ow654qlE2qXTWdeX98FrChJGOs9oUzykv\n\tV+/2J0+tjXevC7gJNu0lCCu3PxfuQs9etWGU9PIM=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200822134036.16209-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 24 Aug 2020 10:32:24 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200822134036.16209-1-laurent.pinchart@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] cam: Limit file write to payload size","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12133,"web_url":"https://patchwork.libcamera.org/comment/12133/","msgid":"<20200824220411.GI594558@oden.dyn.berto.se>","date":"2020-08-24T22:04:11","subject":"Re: [libcamera-devel] [PATCH] cam: Limit file write to payload size","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran, Laurent,\n\nOn 2020-08-24 10:32:24 +0100, Kieran Bingham wrote:\n> Hi Laurent,\n> \n> On 22/08/2020 14:40, Laurent Pinchart wrote:\n> > The payload size in a captured framebuffer is usually equal to the\n> > buffer size. However, for compressed formats, the payload may be\n> > smaller Only write the payload when capturing to a file.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/cam/buffer_writer.cpp | 12 ++++++++++--\n> > 1 file changed, 10 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp\n> > index c5a5eb46224a..6305958924a4 100644\n> > --- a/src/cam/buffer_writer.cpp\n> > +++ b/src/cam/buffer_writer.cpp\n> > @@ -64,9 +64,17 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName)\n> > \tif (fd == -1)\n> > \t\treturn -errno;\n> > \n> > -\tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n> > +\tfor (unsigned int i = 0; i < buffer->planes().size(); ++i) {\n> > +\t\tconst FrameBuffer::Plane &plane = buffer->planes()[i];\n> > +\t\tconst FrameMetadata::Plane &meta = buffer->metadata().planes[i];\n> > +\n> > \t\tvoid *data = mappedBuffers_[plane.fd.fd()].first;\n> > -\t\tunsigned int length = plane.length;\n> > +\t\tunsigned int length = std::min(meta.bytesused, plane.length);\n> > +\n> > +\t\tif (length < meta.bytesused)\n> > +\t\t\tstd::cerr << \"payload size \" << meta.bytesused\n> > +\t\t\t\t << \" smaller than plane size \" << plane.length\n> > +\t\t\t\t << std::endl;\n> \n> Is the cerr necessary? If saving out an MJPEG stream from an MJPEG UVC\n> camera - this would print every frame...\n\nNow I'm confused :-)\n\nWould not the cerr only print if length is set to plane.length while \nmeta.bytesused is larger? I read this as plane.length being the buffer's \nmax allocated size while if the cerr triggers the driver is reporitng \nthat more then the max size is used as bytesused is larger then the \nplane?\n\n> \n> Which is however the use case that makes me believe this patch is useful...\n> \n> So with that considered either way,\n> \n> Reviewed-by: Kieran Bingham \n> \n> > \n> > \t\tret = ::write(fd, data, length);\n> > \t\tif (ret < 0) {\n> > \n> \n> -- \n> Regards\n> --\n> Kieran\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 27046BD87C\n\tfor ;\n\tMon, 24 Aug 2020 22:04:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C81362113;\n\tTue, 25 Aug 2020 00:04:14 +0200 (CEST)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F6646037F\n\tfor ;\n\tTue, 25 Aug 2020 00:04:13 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id m22so11466126ljj.5\n\tfor ;\n\tMon, 24 Aug 2020 15:04:13 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\td10sm2404965ljg.87.2020.08.24.15.04.11\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 24 Aug 2020 15:04:11 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"FWcAe71H\"; dkim-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=nz4FRei8egG8xxogEum5HUyelRZgSi+zt9LdjMWYzNU=;\n\tb=FWcAe71Hx32z6htHEvTmeG6JtfXZQUK/TV/mSMbu1QI/uyqgvnt81xDYSwmFG5PwMa\n\tpJhDQGTddcN7PHf0Lk1gXfj6RtCNEVEXlVFdRTGE0vaRoXC5EJnRdtWBsKtWH8u0QOvc\n\teOMTda3xMqNSYh1kVTS35r/TAR4D4g7TT/ieZIjE8CuUjYuRyBIXu1OUppC5W92Cgk5U\n\tDH8HqwT846LAAcSsw4OCQYrdv/ipA6nmKlS7wPVLN04nq2c0wEZ/ziblvmzn69LjlUev\n\t9DVYwqANAmjqp6sa79qmYyl9rimRkmyDvfxsmhYT6NlLt4z2Da2YUQ5QngTXDZHRdSpD\n\tcmNQ==","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=nz4FRei8egG8xxogEum5HUyelRZgSi+zt9LdjMWYzNU=;\n\tb=h2TDSlNxxVfgIzr5ZzSGnFE6IbJ2vcJYY0NDavaJ4/Oa7uYC0sInLebxrGDSh+RYxO\n\tLCND3QrHuvmPtX5PJSQS5dpA7p9cF1M3bs4m/vUTdnq9M/WPkKfIL795EqPeQw1ghbQl\n\tsDKficIrempFkwoT16rSNLmWrnZnBLjaxHmxlQZsmBzynV5A2daMOFjBcSissS70EyAm\n\tLqKg8z+zdqvCSEUx9uH3UCWOImPKolwGzw9QX9gba5GumCQyoHKOHmhs8HgHoTXjw2J7\n\t6f/v5DUAdWUqOQ300SWMxhSIjmVUonoOgU6Gr7ZCe8jfVHI3GxgyxYa22GauqUUzyNrh\n\tKSeg==","X-Gm-Message-State":"AOAM531r1zQ7kemOeKUULKO1l5+bGZgJpO49bR7Rd+2gim59us6fKItY\n\tdT1zXg7Qh8ffZtbS5a2f4IGvVWId+bRQdA==","X-Google-Smtp-Source":"ABdhPJxQYnmfZw/99z/o/KG7d4MyW64t9XmqZniCFd5y7B2/9cmtfqIZv1QJBk2bknlI0vGg3YMICw==","X-Received":"by 2002:a2e:545:: with SMTP id 66mr3677365ljf.285.1598306652557; \n\tMon, 24 Aug 2020 15:04:12 -0700 (PDT)","Date":"Tue, 25 Aug 2020 00:04:11 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Message-ID":"<20200824220411.GI594558@oden.dyn.berto.se>","References":"<20200822134036.16209-1-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] cam: Limit file write to payload size","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12137,"web_url":"https://patchwork.libcamera.org/comment/12137/","msgid":"<20200824225309.GM6002@pendragon.ideasonboard.com>","date":"2020-08-24T22:53:09","subject":"Re: [libcamera-devel] [PATCH] cam: Limit file write to payload size","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 25, 2020 at 12:04:11AM +0200, Niklas Söderlund wrote:\n> On 2020-08-24 10:32:24 +0100, Kieran Bingham wrote:\n> > On 22/08/2020 14:40, Laurent Pinchart wrote:\n> > > The payload size in a captured framebuffer is usually equal to the\n> > > buffer size. However, for compressed formats, the payload may be\n> > > smaller Only write the payload when capturing to a file.\n> > > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/cam/buffer_writer.cpp | 12 ++++++++++--\n> > > 1 file changed, 10 insertions(+), 2 deletions(-)\n> > > \n> > > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp\n> > > index c5a5eb46224a..6305958924a4 100644\n> > > --- a/src/cam/buffer_writer.cpp\n> > > +++ b/src/cam/buffer_writer.cpp\n> > > @@ -64,9 +64,17 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName)\n> > > \tif (fd == -1)\n> > > \t\treturn -errno;\n> > > \n> > > -\tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n> > > +\tfor (unsigned int i = 0; i < buffer->planes().size(); ++i) {\n> > > +\t\tconst FrameBuffer::Plane &plane = buffer->planes()[i];\n> > > +\t\tconst FrameMetadata::Plane &meta = buffer->metadata().planes[i];\n> > > +\n> > > \t\tvoid *data = mappedBuffers_[plane.fd.fd()].first;\n> > > -\t\tunsigned int length = plane.length;\n> > > +\t\tunsigned int length = std::min(meta.bytesused, plane.length);\n> > > +\n> > > +\t\tif (length < meta.bytesused)\n> > > +\t\t\tstd::cerr << \"payload size \" << meta.bytesused\n> > > +\t\t\t\t << \" smaller than plane size \" << plane.length\n> > > +\t\t\t\t << std::endl;\n> > \n> > Is the cerr necessary? If saving out an MJPEG stream from an MJPEG UVC\n> > camera - this would print every frame...\n> \n> Now I'm confused :-)\n> \n> Would not the cerr only print if length is set to plane.length while \n> meta.bytesused is larger? I read this as plane.length being the buffer's \n> max allocated size while if the cerr triggers the driver is reporitng \n> that more then the max size is used as bytesused is larger then the \n> plane?\n\nThe check is correct, but the message isn't. And the check could\nactually be be made clearer.\n\n\t\tif (meta.bytesused > plane.length)\n\t\t\tstd::cerr << \"payload size \" << meta.bytesused\n\t\t\t\t << \" larger than plane size \" << plane.length\n\t\t\t\t << std::endl;\n\n> > Which is however the use case that makes me believe this patch is useful...\n> > \n> > So with that considered either way,\n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > > \n> > > \t\tret = ::write(fd, data, length);\n> > > \t\tif (ret < 0) {\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 4F46DBD87C\n\tfor ;\n\tMon, 24 Aug 2020 22:53:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D11A762818;\n\tTue, 25 Aug 2020 00:53:30 +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 026D76037F\n\tfor ;\n\tTue, 25 Aug 2020 00:53:28 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 740369CE;\n\tTue, 25 Aug 2020 00:53:28 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Vib/Xwqy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598309608;\n\tbh=9fvOCdYNx2k21rKpEjTRQLgEF6qfhTrcQIkflb5vHYs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Vib/Xwqy8KhDv8wMz7SRaJsc4Sn+DVVdevc9foDWId982cVN9NjnXXlInx9OzNLLs\n\tDCH5ID2SNdGyYwK/0BT5Qv/Kl8LieRwKzi82/r3rwwzTQOHZPLQ7F+ayO7iJyoEa6y\n\tmdshgfU4KsZu4HTQ6ZyUYYEi4TLkEAhv2gmh2MSQ=","Date":"Tue, 25 Aug 2020 01:53:09 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200824225309.GM6002@pendragon.ideasonboard.com>","References":"<20200822134036.16209-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20200824220411.GI594558@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200824220411.GI594558@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH] cam: Limit file write to payload size","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12143,"web_url":"https://patchwork.libcamera.org/comment/12143/","msgid":"","date":"2020-08-25T08:50:21","subject":"Re: [libcamera-devel] [PATCH] cam: Limit file write to payload size","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 24/08/2020 23:53, Laurent Pinchart wrote:\n> On Tue, Aug 25, 2020 at 12:04:11AM +0200, Niklas Söderlund wrote:\n>> On 2020-08-24 10:32:24 +0100, Kieran Bingham wrote:\n>>> On 22/08/2020 14:40, Laurent Pinchart wrote:\n>>>> The payload size in a captured framebuffer is usually equal to the\n>>>> buffer size. However, for compressed formats, the payload may be\n>>>> smaller Only write the payload when capturing to a file.\n>>>>\n>>>> Signed-off-by: Laurent Pinchart \n>>>> ---\n>>>> src/cam/buffer_writer.cpp | 12 ++++++++++--\n>>>> 1 file changed, 10 insertions(+), 2 deletions(-)\n>>>>\n>>>> diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp\n>>>> index c5a5eb46224a..6305958924a4 100644\n>>>> --- a/src/cam/buffer_writer.cpp\n>>>> +++ b/src/cam/buffer_writer.cpp\n>>>> @@ -64,9 +64,17 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName)\n>>>> \tif (fd == -1)\n>>>> \t\treturn -errno;\n>>>> \n>>>> -\tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n>>>> +\tfor (unsigned int i = 0; i < buffer->planes().size(); ++i) {\n>>>> +\t\tconst FrameBuffer::Plane &plane = buffer->planes()[i];\n>>>> +\t\tconst FrameMetadata::Plane &meta = buffer->metadata().planes[i];\n>>>> +\n>>>> \t\tvoid *data = mappedBuffers_[plane.fd.fd()].first;\n>>>> -\t\tunsigned int length = plane.length;\n>>>> +\t\tunsigned int length = std::min(meta.bytesused, plane.length);\n>>>> +\n>>>> +\t\tif (length < meta.bytesused)\n>>>> +\t\t\tstd::cerr << \"payload size \" << meta.bytesused\n>>>> +\t\t\t\t << \" smaller than plane size \" << plane.length\n>>>> +\t\t\t\t << std::endl;\n>>>\n>>> Is the cerr necessary? If saving out an MJPEG stream from an MJPEG UVC\n>>> camera - this would print every frame...\n>>\n>> Now I'm confused :-)\n>>\n>> Would not the cerr only print if length is set to plane.length while \n>> meta.bytesused is larger? I read this as plane.length being the buffer's \n>> max allocated size while if the cerr triggers the driver is reporitng \n>> that more then the max size is used as bytesused is larger then the \n>> plane?\n> \n> The check is correct, but the message isn't. And the check could\n> actually be be made clearer.\n> \n> \t\tif (meta.bytesused > plane.length)\n> \t\t\tstd::cerr << \"payload size \" << meta.bytesused\n> \t\t\t\t << \" larger than plane size \" << plane.length\n> \t\t\t\t << std::endl;\n\n\nAhh, now that would be a lot easier to parse.\n\n+1\n\n--\nKieran\n\n\n> \n>>> Which is however the use case that makes me believe this patch is useful...\n>>>\n>>> So with that considered either way,\n>>>\n>>> Reviewed-by: Kieran Bingham \n>>>\n>>>> \n>>>> \t\tret = ::write(fd, data, length);\n>>>> \t\tif (ret < 0) {\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 9A218BD87C\n\tfor ;\n\tTue, 25 Aug 2020 08:50:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 075A862851;\n\tTue, 25 Aug 2020 10:50:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 78E4C6037B\n\tfor ;\n\tTue, 25 Aug 2020 10:50:28 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B8CF329E;\n\tTue, 25 Aug 2020 10:50:23 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KvjVihff\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598345423;\n\tbh=EGCq+H0J0MoHUSRfH35ffnSJHI5+Eda2thI1onm449k=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=KvjVihff0MWkPeLtQjAVzbyNSiKOX6gPpxd84/kM7Ap9hck2K7NWEVjBya2QzNzYy\n\t5H6Tj+ZdXp6iCwjHWqUC1Gv2z9xQ+wd1RAhZ+pHb4crDBNxaFGqqc0SO5t0v7oJjwW\n\tVveeLXX1RXsVtPdCAVp2J3YPce0moLg5emuHhZqs=","To":"Laurent Pinchart , =?utf-8?q?Niklas?=\n\t=?utf-8?q?_S=C3=B6derlund?= ","References":"<20200822134036.16209-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20200824220411.GI594558@oden.dyn.berto.se>\n\t<20200824225309.GM6002@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Tue, 25 Aug 2020 09:50:21 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200824225309.GM6002@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] cam: Limit file write to payload size","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]