[{"id":12990,"web_url":"https://patchwork.libcamera.org/comment/12990/","msgid":"","date":"2020-10-05T16:31:31","subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 05/10/2020 11:46, Laurent Pinchart wrote:\n> From: Jacopo Mondi \n> \n> A CameraStream maps a stream requested by Android to a\n> libcamera::StreamConfiguration. It shall then be constructed with\n> those information clearly specified.\n> > Change the CameraStream constructor to accept an Android\n> camera3_stream_t and a libcamera::StreamConfiguration to copy the\n> format and size information in the class members as no reference can be\n> taken to the StreamConfiguration instances as they're subject to\n> relocations.\n\nI'm not sure we need to say all that, but could have just been \"Pass the\nandroid camera3_stream_t, and a libcamera::StreamConfiguration to\nidentify the source and destination parameters of this stream.\n\n> \n> Pass to the CameraStream constructor a pointer to the CameraDevice class\n> and make the CameraConfiguration accessible. It will be used to retrieve\n> the StreamConfiguration associated with the CameraStream.\n\nPass a CameraDevice pointer to the CameraStream constructor to allow\nretrieval of the StreamConfiguration associated with the CameraStream.\n\n> Also change the format on which the CameraDevice performs checks to\n> decide if post-processing is required, as the libcamera facing format is\n> not meaningful anymore, but the Android required format should be used\n> instead.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 11 +++++------\n> src/android/camera_device.h | 4 ++++\n> src/android/camera_stream.cpp | 14 ++++++++++++--\n> src/android/camera_stream.h | 18 ++++++++++++++++--\n> 4 files changed, 37 insertions(+), 10 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 9c9a5cfa3c2f..2c4dd4dee28c 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1216,8 +1216,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \n> \t\tconfig_->addConfiguration(streamConfiguration);\n> \t\tunsigned int index = config_->size() - 1;\n> -\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n> -\t\t\t\t index);\n> +\t\tstreams_.emplace_back(this, stream, streamConfiguration,\n> +\t\t\t\t CameraStream::Type::Direct, index);\n> \t\tstream->priv = static_cast(&streams_.back());\n> \t}\n> \n> @@ -1272,8 +1272,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t}\n> \n> \t\tStreamConfiguration &cfg = config_->at(index);\n> -\n> -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index);\n> +\t\tstreams_.emplace_back(this, jpegStream, cfg, type, index);\n> \t\tjpegStream->priv = static_cast(&streams_.back());\n> \t}\n> \n> @@ -1405,7 +1404,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n> \n> \t\t/* Software streams are handled after hardware streams complete. */\n> -\t\tif (cameraStream->format() == formats::MJPEG)\n> +\t\tif (cameraStream->outputFormat() == HAL_PIXEL_FORMAT_BLOB)\n\nI understand the premise for this change, it's just so unfortunate that\nthe android format is so ... well ... unrepresentative :-)\n\n\nI haven't gone to check yet, but I wonder if there is a better boolean\nlogic to check ...\n\n\t\tif (format != NV12) perhaps ?\n\nI guess it depends on what other combinations we'll see through here.\n\nBut no need to change this now\n\n> \t\t\tcontinue;\n> \n> \t\t/*\n> @@ -1469,7 +1468,7 @@ void CameraDevice::requestComplete(Request *request)\n> \t\tCameraStream *cameraStream =\n> \t\t\tstatic_cast(descriptor->buffers[i].stream->priv);\n> \n> -\t\tif (cameraStream->format() != formats::MJPEG)\n> +\t\tif (cameraStream->outputFormat() != HAL_PIXEL_FORMAT_BLOB)\n> \t\t\tcontinue;\n> \n> \t\tEncoder *encoder = cameraStream->encoder();\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 52923ec979a7..4e326fa3e1fb 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -43,6 +43,10 @@ public:\n> \tunsigned int id() const { return id_; }\n> \tcamera3_device_t *camera3Device() { return &camera3Device_; }\n> \tconst libcamera::Camera *camera() const { return camera_.get(); }\n> +\tlibcamera::CameraConfiguration *cameraConfiguration() const\n> +\t{\n> +\t\treturn config_.get();\n> +\t}\n> \n> \tint facing() const { return facing_; }\n> \tint orientation() const { return orientation_; }\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 5b2b625c563b..5c1f1d7da416 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -7,13 +7,23 @@\n> \n> #include \"camera_stream.h\"\n> \n> +#include \"camera_device.h\"\n> #include \"jpeg/encoder_libjpeg.h\"\n> \n> using namespace libcamera;\n> \n> -CameraStream::CameraStream(PixelFormat format, Size size, Type type, unsigned int index)\n> -\t: format_(format), size_(size), type_(type), index_(index), encoder(nullptr)\n> +CameraStream::CameraStream(CameraDevice *cameraDev,\n> +\t\t\t camera3_stream_t *androidStream,\n> +\t\t\t const libcamera::StreamConfiguration &cfg,\n> +\t\t\t Type type, unsigned int index)\n> +\t: cameraDevice_(cameraDev), androidStream_(androidStream), type_(type),\n> +\t index_(index), encoder_(nullptr)\n> {\n> +\tconfig_ = cameraDevice_->cameraConfiguration();\n> +\n> +\tformat_ = cfg.pixelFormat;\n> +\tsize_ = cfg.size;\n> +\n> \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> \t\tencoder_.reset(new EncoderLibJpeg);\n> }\n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index d0dc40d81151..2d71a50c78a4 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -9,11 +9,16 @@\n> \n> #include \n> \n> +#include \n> +\n> +#include \n> #include \n> #include \n> \n> #include \"jpeg/encoder.h\"\n> \n> +class CameraDevice;\n> +\n> class CameraStream\n> {\n> public:\n> @@ -99,9 +104,12 @@ public:\n> \t\tInternal,\n> \t\tMapped,\n> \t};\n> -\tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> +\tCameraStream(CameraDevice *cameraDev,\n\nI would have just used 'dev', or 'device', I don't think it conflicts\nwith anything else in this function does it ?\n\nor as the class private member it goes into is cameraDevice_, it could\nhave been cameraDevice ;-) - But it's not important either.\n\nReviewed-by: Kieran Bingham \n\n\n\n> +\t\t camera3_stream_t *androidStream,\n> +\t\t const libcamera::StreamConfiguration &cfg,\n> \t\t Type type, unsigned int index);\n> \n> +\tint outputFormat() const { return androidStream_->format; }\n> \tconst libcamera::PixelFormat &format() const { return format_; }\n> \tconst libcamera::Size &size() const { return size_; }\n> \tType type() const { return type_; }\n> @@ -111,9 +119,15 @@ public:\n> \tint configure(const libcamera::StreamConfiguration &cfg);\n> \n> private:\n> +\tCameraDevice *cameraDevice_;\n> +\tlibcamera::CameraConfiguration *config_;\n> +\tcamera3_stream_t *androidStream_;\n> +\tType type_;\n> +\n> +\t/* Libcamera facing format and sizes. */\n> \tlibcamera::PixelFormat format_;\n> \tlibcamera::Size size_;\n> -\tType type_;\n> +\n> \t/*\n> \t * The index of the libcamera StreamConfiguration as added during\n> \t * configureStreams(). A single libcamera Stream may be used to deliver\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 880F0C3B5D\n\tfor ;\n\tMon, 5 Oct 2020 16:31:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 236A963BFE;\n\tMon, 5 Oct 2020 18:31:36 +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 AEFDC63BED\n\tfor ;\n\tMon, 5 Oct 2020 18:31:34 +0200 (CEST)","from [192.168.0.217]\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 3190F3B;\n\tMon, 5 Oct 2020 18:31:34 +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=\"bc/86CQ/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601915494;\n\tbh=lKgdLTCj6ATBZpJxAKyYzTi5Qkqi9KJVhN6k8AsbHw4=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=bc/86CQ/a6i1EnlyC0Pd8ZkNpK0brH9btvIDgQ3wQeGkiK0QOiGvH24qyaqBF6rU1\n\tGT5zHLEOmxuSFbOjk11Qa58k45Ld7Bq+2T40HALxTjt8qO5nV6AT5vJjPTy8+r3yqS\n\tWA7hCxlzifkzw65EU7Xsr8zqeEX+7sR2vgs/l8UU=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20201005104649.10812-1-laurent.pinchart@ideasonboard.com>\n\t<20201005104649.10812-5-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, 5 Oct 2020 17:31:31 +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":"<20201005104649.10812-5-laurent.pinchart@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","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":12992,"web_url":"https://patchwork.libcamera.org/comment/12992/","msgid":"<20201005164055.GV3931@pendragon.ideasonboard.com>","date":"2020-10-05T16:40:55","subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Oct 05, 2020 at 05:31:31PM +0100, Kieran Bingham wrote:\n> On 05/10/2020 11:46, Laurent Pinchart wrote:\n> > From: Jacopo Mondi \n> > \n> > A CameraStream maps a stream requested by Android to a\n> > libcamera::StreamConfiguration. It shall then be constructed with\n> > those information clearly specified.\n> > > Change the CameraStream constructor to accept an Android\n> > camera3_stream_t and a libcamera::StreamConfiguration to copy the\n> > format and size information in the class members as no reference can be\n> > taken to the StreamConfiguration instances as they're subject to\n> > relocations.\n> \n> I'm not sure we need to say all that, but could have just been \"Pass the\n> android camera3_stream_t, and a libcamera::StreamConfiguration to\n> identify the source and destination parameters of this stream.\n> \n> > Pass to the CameraStream constructor a pointer to the CameraDevice class\n> > and make the CameraConfiguration accessible. It will be used to retrieve\n> > the StreamConfiguration associated with the CameraStream.\n> \n> Pass a CameraDevice pointer to the CameraStream constructor to allow\n> retrieval of the StreamConfiguration associated with the CameraStream.\n> \n> > Also change the format on which the CameraDevice performs checks to\n> > decide if post-processing is required, as the libcamera facing format is\n> > not meaningful anymore, but the Android required format should be used\n> > instead.\n> > \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 11 +++++------\n> > src/android/camera_device.h | 4 ++++\n> > src/android/camera_stream.cpp | 14 ++++++++++++--\n> > src/android/camera_stream.h | 18 ++++++++++++++++--\n> > 4 files changed, 37 insertions(+), 10 deletions(-)\n> > \n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 9c9a5cfa3c2f..2c4dd4dee28c 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1216,8 +1216,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > \n> > \t\tconfig_->addConfiguration(streamConfiguration);\n> > \t\tunsigned int index = config_->size() - 1;\n> > -\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n> > -\t\t\t\t index);\n> > +\t\tstreams_.emplace_back(this, stream, streamConfiguration,\n> > +\t\t\t\t CameraStream::Type::Direct, index);\n> > \t\tstream->priv = static_cast(&streams_.back());\n> > \t}\n> > \n> > @@ -1272,8 +1272,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > \t\t}\n> > \n> > \t\tStreamConfiguration &cfg = config_->at(index);\n> > -\n> > -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index);\n> > +\t\tstreams_.emplace_back(this, jpegStream, cfg, type, index);\n> > \t\tjpegStream->priv = static_cast(&streams_.back());\n> > \t}\n> > \n> > @@ -1405,7 +1404,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n> > \n> > \t\t/* Software streams are handled after hardware streams complete. */\n> > -\t\tif (cameraStream->format() == formats::MJPEG)\n> > +\t\tif (cameraStream->outputFormat() == HAL_PIXEL_FORMAT_BLOB)\n> \n> I understand the premise for this change, it's just so unfortunate that\n> the android format is so ... well ... unrepresentative :-)\n> \n> \n> I haven't gone to check yet, but I wonder if there is a better boolean\n> logic to check ...\n> \n> \t\tif (format != NV12) perhaps ?\n> \n> I guess it depends on what other combinations we'll see through here.\n> \n> But no need to change this now\n> \n> > \t\t\tcontinue;\n> > \n> > \t\t/*\n> > @@ -1469,7 +1468,7 @@ void CameraDevice::requestComplete(Request *request)\n> > \t\tCameraStream *cameraStream =\n> > \t\t\tstatic_cast(descriptor->buffers[i].stream->priv);\n> > \n> > -\t\tif (cameraStream->format() != formats::MJPEG)\n> > +\t\tif (cameraStream->outputFormat() != HAL_PIXEL_FORMAT_BLOB)\n> > \t\t\tcontinue;\n> > \n> > \t\tEncoder *encoder = cameraStream->encoder();\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 52923ec979a7..4e326fa3e1fb 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -43,6 +43,10 @@ public:\n> > \tunsigned int id() const { return id_; }\n> > \tcamera3_device_t *camera3Device() { return &camera3Device_; }\n> > \tconst libcamera::Camera *camera() const { return camera_.get(); }\n> > +\tlibcamera::CameraConfiguration *cameraConfiguration() const\n> > +\t{\n> > +\t\treturn config_.get();\n> > +\t}\n> > \n> > \tint facing() const { return facing_; }\n> > \tint orientation() const { return orientation_; }\n> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > index 5b2b625c563b..5c1f1d7da416 100644\n> > --- a/src/android/camera_stream.cpp\n> > +++ b/src/android/camera_stream.cpp\n> > @@ -7,13 +7,23 @@\n> > \n> > #include \"camera_stream.h\"\n> > \n> > +#include \"camera_device.h\"\n> > #include \"jpeg/encoder_libjpeg.h\"\n> > \n> > using namespace libcamera;\n> > \n> > -CameraStream::CameraStream(PixelFormat format, Size size, Type type, unsigned int index)\n> > -\t: format_(format), size_(size), type_(type), index_(index), encoder(nullptr)\n> > +CameraStream::CameraStream(CameraDevice *cameraDev,\n> > +\t\t\t camera3_stream_t *androidStream,\n> > +\t\t\t const libcamera::StreamConfiguration &cfg,\n> > +\t\t\t Type type, unsigned int index)\n> > +\t: cameraDevice_(cameraDev), androidStream_(androidStream), type_(type),\n> > +\t index_(index), encoder_(nullptr)\n> > {\n> > +\tconfig_ = cameraDevice_->cameraConfiguration();\n> > +\n> > +\tformat_ = cfg.pixelFormat;\n> > +\tsize_ = cfg.size;\n> > +\n> > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > \t\tencoder_.reset(new EncoderLibJpeg);\n> > }\n> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > index d0dc40d81151..2d71a50c78a4 100644\n> > --- a/src/android/camera_stream.h\n> > +++ b/src/android/camera_stream.h\n> > @@ -9,11 +9,16 @@\n> > \n> > #include \n> > \n> > +#include \n> > +\n> > +#include \n> > #include \n> > #include \n> > \n> > #include \"jpeg/encoder.h\"\n> > \n> > +class CameraDevice;\n> > +\n> > class CameraStream\n> > {\n> > public:\n> > @@ -99,9 +104,12 @@ public:\n> > \t\tInternal,\n> > \t\tMapped,\n> > \t};\n> > -\tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> > +\tCameraStream(CameraDevice *cameraDev,\n> \n> I would have just used 'dev', or 'device', I don't think it conflicts\n> with anything else in this function does it ?\n\nI like short names when they're not ambiguous, and even more\nimportantly, when they're used consistently. If someone wants to go\nthrough the HAL code at some point to propose a consistent naming scheme\n(CameraStream and camera3_stream_t is a good candidate for instance) and\nmake sure it's consistently applied, I'll be happy to review it.\n\n> or as the class private member it goes into is cameraDevice_, it could\n> have been cameraDevice ;-) - But it's not important either.\n> \n> Reviewed-by: Kieran Bingham \n> \n> > +\t\t camera3_stream_t *androidStream,\n> > +\t\t const libcamera::StreamConfiguration &cfg,\n> > \t\t Type type, unsigned int index);\n> > \n> > +\tint outputFormat() const { return androidStream_->format; }\n> > \tconst libcamera::PixelFormat &format() const { return format_; }\n> > \tconst libcamera::Size &size() const { return size_; }\n> > \tType type() const { return type_; }\n> > @@ -111,9 +119,15 @@ public:\n> > \tint configure(const libcamera::StreamConfiguration &cfg);\n> > \n> > private:\n> > +\tCameraDevice *cameraDevice_;\n> > +\tlibcamera::CameraConfiguration *config_;\n> > +\tcamera3_stream_t *androidStream_;\n> > +\tType type_;\n> > +\n> > +\t/* Libcamera facing format and sizes. */\n> > \tlibcamera::PixelFormat format_;\n> > \tlibcamera::Size size_;\n> > -\tType type_;\n> > +\n> > \t/*\n> > \t * The index of the libcamera StreamConfiguration as added during\n> > \t * configureStreams(). A single libcamera Stream may be used to deliver","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 D7FE4C3B5C\n\tfor ;\n\tMon, 5 Oct 2020 16:41:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 634DF63C81;\n\tMon, 5 Oct 2020 18:41:36 +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 0543F63BED\n\tfor ;\n\tMon, 5 Oct 2020 18:41:35 +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 5A6983B;\n\tMon, 5 Oct 2020 18:41:34 +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=\"SmJ5N6h1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601916094;\n\tbh=SHFi7A/CIfJ8r+cvji0TrZFwMu8HpCaHt9ubpqzvqZE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SmJ5N6h1B4yqb6WLlqYJIqhMobZdT27KtAbfAyQToNBPJrf0mL1HDAZK0z/3Rw7cg\n\tOIHaHKBeL2a8FtuSon/iahEZ8+A8kt1w+yHBbCKQqsUf8Fdf5YPR3nvmoJtns+5S0x\n\tDPBlHjcnLwKWGFMmw+/buUURlZULiTSTo0lL5gL8=","Date":"Mon, 5 Oct 2020 19:40:55 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201005164055.GV3931@pendragon.ideasonboard.com>","References":"<20201005104649.10812-1-laurent.pinchart@ideasonboard.com>\n\t<20201005104649.10812-5-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13008,"web_url":"https://patchwork.libcamera.org/comment/13008/","msgid":"<20201006112449.bvjxwkqmz2fnuwkq@uno.localdomain>","date":"2020-10-06T11:24:49","subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Mon, Oct 05, 2020 at 05:31:31PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 05/10/2020 11:46, Laurent Pinchart wrote:\n> > From: Jacopo Mondi \n> >\n> > A CameraStream maps a stream requested by Android to a\n> > libcamera::StreamConfiguration. It shall then be constructed with\n> > those information clearly specified.\n> > > Change the CameraStream constructor to accept an Android\n> > camera3_stream_t and a libcamera::StreamConfiguration to copy the\n> > format and size information in the class members as no reference can be\n> > taken to the StreamConfiguration instances as they're subject to\n> > relocations.\n>\n> I'm not sure we need to say all that, but could have just been \"Pass the\n> android camera3_stream_t, and a libcamera::StreamConfiguration to\n> identify the source and destination parameters of this stream.\n>\n> >\n> > Pass to the CameraStream constructor a pointer to the CameraDevice class\n> > and make the CameraConfiguration accessible. It will be used to retrieve\n> > the StreamConfiguration associated with the CameraStream.\n>\n> Pass a CameraDevice pointer to the CameraStream constructor to allow\n> retrieval of the StreamConfiguration associated with the CameraStream.\n>\n\nThanks, much better!\n\n> > Also change the format on which the CameraDevice performs checks to\n> > decide if post-processing is required, as the libcamera facing format is\n> > not meaningful anymore, but the Android required format should be used\n> > instead.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 11 +++++------\n> > src/android/camera_device.h | 4 ++++\n> > src/android/camera_stream.cpp | 14 ++++++++++++--\n> > src/android/camera_stream.h | 18 ++++++++++++++++--\n> > 4 files changed, 37 insertions(+), 10 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 9c9a5cfa3c2f..2c4dd4dee28c 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1216,8 +1216,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >\n> > \t\tconfig_->addConfiguration(streamConfiguration);\n> > \t\tunsigned int index = config_->size() - 1;\n> > -\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n> > -\t\t\t\t index);\n> > +\t\tstreams_.emplace_back(this, stream, streamConfiguration,\n> > +\t\t\t\t CameraStream::Type::Direct, index);\n> > \t\tstream->priv = static_cast(&streams_.back());\n> > \t}\n> >\n> > @@ -1272,8 +1272,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > \t\t}\n> >\n> > \t\tStreamConfiguration &cfg = config_->at(index);\n> > -\n> > -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index);\n> > +\t\tstreams_.emplace_back(this, jpegStream, cfg, type, index);\n> > \t\tjpegStream->priv = static_cast(&streams_.back());\n> > \t}\n> >\n> > @@ -1405,7 +1404,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n> >\n> > \t\t/* Software streams are handled after hardware streams complete. */\n> > -\t\tif (cameraStream->format() == formats::MJPEG)\n> > +\t\tif (cameraStream->outputFormat() == HAL_PIXEL_FORMAT_BLOB)\n>\n> I understand the premise for this change, it's just so unfortunate that\n> the android format is so ... well ... unrepresentative :-)\n\nMore than this, I'm thinking of doing what I'll do with the libcamera\nStreamConfiguration\n if (cameraStream->androidStream().format == ... )\n\nAnd not provide a confusing CameraStream::outputFormat() helper at\nall.\n\n>\n>\n> I haven't gone to check yet, but I wonder if there is a better boolean\n> logic to check ...\n>\n> \t\tif (format != NV12) perhaps ?\n\nWe can't bet to have only NV12 and MJPEG\n\n>\n> I guess it depends on what other combinations we'll see through here.\n>\n> But no need to change this now\n>\n> > \t\t\tcontinue;\n> >\n> > \t\t/*\n> > @@ -1469,7 +1468,7 @@ void CameraDevice::requestComplete(Request *request)\n> > \t\tCameraStream *cameraStream =\n> > \t\t\tstatic_cast(descriptor->buffers[i].stream->priv);\n> >\n> > -\t\tif (cameraStream->format() != formats::MJPEG)\n> > +\t\tif (cameraStream->outputFormat() != HAL_PIXEL_FORMAT_BLOB)\n> > \t\t\tcontinue;\n> >\n> > \t\tEncoder *encoder = cameraStream->encoder();\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 52923ec979a7..4e326fa3e1fb 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -43,6 +43,10 @@ public:\n> > \tunsigned int id() const { return id_; }\n> > \tcamera3_device_t *camera3Device() { return &camera3Device_; }\n> > \tconst libcamera::Camera *camera() const { return camera_.get(); }\n> > +\tlibcamera::CameraConfiguration *cameraConfiguration() const\n> > +\t{\n> > +\t\treturn config_.get();\n> > +\t}\n> >\n> > \tint facing() const { return facing_; }\n> > \tint orientation() const { return orientation_; }\n> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > index 5b2b625c563b..5c1f1d7da416 100644\n> > --- a/src/android/camera_stream.cpp\n> > +++ b/src/android/camera_stream.cpp\n> > @@ -7,13 +7,23 @@\n> >\n> > #include \"camera_stream.h\"\n> >\n> > +#include \"camera_device.h\"\n> > #include \"jpeg/encoder_libjpeg.h\"\n> >\n> > using namespace libcamera;\n> >\n> > -CameraStream::CameraStream(PixelFormat format, Size size, Type type, unsigned int index)\n> > -\t: format_(format), size_(size), type_(type), index_(index), encoder(nullptr)\n> > +CameraStream::CameraStream(CameraDevice *cameraDev,\n> > +\t\t\t camera3_stream_t *androidStream,\n> > +\t\t\t const libcamera::StreamConfiguration &cfg,\n> > +\t\t\t Type type, unsigned int index)\n> > +\t: cameraDevice_(cameraDev), androidStream_(androidStream), type_(type),\n> > +\t index_(index), encoder_(nullptr)\n> > {\n> > +\tconfig_ = cameraDevice_->cameraConfiguration();\n> > +\n> > +\tformat_ = cfg.pixelFormat;\n> > +\tsize_ = cfg.size;\n> > +\n> > \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> > \t\tencoder_.reset(new EncoderLibJpeg);\n> > }\n> > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > index d0dc40d81151..2d71a50c78a4 100644\n> > --- a/src/android/camera_stream.h\n> > +++ b/src/android/camera_stream.h\n> > @@ -9,11 +9,16 @@\n> >\n> > #include \n> >\n> > +#include \n> > +\n> > +#include \n> > #include \n> > #include \n> >\n> > #include \"jpeg/encoder.h\"\n> >\n> > +class CameraDevice;\n> > +\n> > class CameraStream\n> > {\n> > public:\n> > @@ -99,9 +104,12 @@ public:\n> > \t\tInternal,\n> > \t\tMapped,\n> > \t};\n> > -\tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> > +\tCameraStream(CameraDevice *cameraDev,\n>\n> I would have just used 'dev', or 'device', I don't think it conflicts\n> with anything else in this function does it ?\n>\n> or as the class private member it goes into is cameraDevice_, it could\n> have been cameraDevice ;-) - But it's not important either.\n\nI thought about shortening the name too, but as it goes to\ncameraDevice_ I kept it long.\n\nAnd as Laurent said, if we could unify the naming schemes (ie.\neverything android prefixed with camera3 and everything libcamera\nprefixed with.... ) it would be nice\n\n>\n> Reviewed-by: Kieran Bingham \n\nThanks\n j\n\n>\n>\n>\n> > +\t\t camera3_stream_t *androidStream,\n> > +\t\t const libcamera::StreamConfiguration &cfg,\n> > \t\t Type type, unsigned int index);\n> >\n> > +\tint outputFormat() const { return androidStream_->format; }\n> > \tconst libcamera::PixelFormat &format() const { return format_; }\n> > \tconst libcamera::Size &size() const { return size_; }\n> > \tType type() const { return type_; }\n> > @@ -111,9 +119,15 @@ public:\n> > \tint configure(const libcamera::StreamConfiguration &cfg);\n> >\n> > private:\n> > +\tCameraDevice *cameraDevice_;\n> > +\tlibcamera::CameraConfiguration *config_;\n> > +\tcamera3_stream_t *androidStream_;\n> > +\tType type_;\n> > +\n> > +\t/* Libcamera facing format and sizes. */\n> > \tlibcamera::PixelFormat format_;\n> > \tlibcamera::Size size_;\n> > -\tType type_;\n> > +\n> > \t/*\n> > \t * The index of the libcamera StreamConfiguration as added during\n> > \t * configureStreams(). A single libcamera Stream may be used to deliver\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 31F9DBEEE0\n\tfor ;\n\tTue, 6 Oct 2020 11:20:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E80463C13;\n\tTue, 6 Oct 2020 13:20:51 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7C81860363\n\tfor ;\n\tTue, 6 Oct 2020 13:20:50 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id EAE521C0006;\n\tTue, 6 Oct 2020 11:20:49 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Tue, 6 Oct 2020 13:24:49 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20201006112449.bvjxwkqmz2fnuwkq@uno.localdomain>","References":"<20201005104649.10812-1-laurent.pinchart@ideasonboard.com>\n\t<20201005104649.10812-5-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13010,"web_url":"https://patchwork.libcamera.org/comment/13010/","msgid":"","date":"2020-10-06T12:21:00","subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 06/10/2020 12:24, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Mon, Oct 05, 2020 at 05:31:31PM +0100, Kieran Bingham wrote:\n>> Hi Jacopo,\n>>\n>> On 05/10/2020 11:46, Laurent Pinchart wrote:\n>>> From: Jacopo Mondi \n>>>\n>>> A CameraStream maps a stream requested by Android to a\n>>> libcamera::StreamConfiguration. It shall then be constructed with\n>>> those information clearly specified.\n>>>> Change the CameraStream constructor to accept an Android\n>>> camera3_stream_t and a libcamera::StreamConfiguration to copy the\n>>> format and size information in the class members as no reference can be\n>>> taken to the StreamConfiguration instances as they're subject to\n>>> relocations.\n>>\n>> I'm not sure we need to say all that, but could have just been \"Pass the\n>> android camera3_stream_t, and a libcamera::StreamConfiguration to\n>> identify the source and destination parameters of this stream.\n>>\n>>>\n>>> Pass to the CameraStream constructor a pointer to the CameraDevice class\n>>> and make the CameraConfiguration accessible. It will be used to retrieve\n>>> the StreamConfiguration associated with the CameraStream.\n>>\n>> Pass a CameraDevice pointer to the CameraStream constructor to allow\n>> retrieval of the StreamConfiguration associated with the CameraStream.\n>>\n> \n> Thanks, much better!\n> \n>>> Also change the format on which the CameraDevice performs checks to\n>>> decide if post-processing is required, as the libcamera facing format is\n>>> not meaningful anymore, but the Android required format should be used\n>>> instead.\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/android/camera_device.cpp | 11 +++++------\n>>> src/android/camera_device.h | 4 ++++\n>>> src/android/camera_stream.cpp | 14 ++++++++++++--\n>>> src/android/camera_stream.h | 18 ++++++++++++++++--\n>>> 4 files changed, 37 insertions(+), 10 deletions(-)\n>>>\n>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>> index 9c9a5cfa3c2f..2c4dd4dee28c 100644\n>>> --- a/src/android/camera_device.cpp\n>>> +++ b/src/android/camera_device.cpp\n>>> @@ -1216,8 +1216,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>>\n>>> \t\tconfig_->addConfiguration(streamConfiguration);\n>>> \t\tunsigned int index = config_->size() - 1;\n>>> -\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n>>> -\t\t\t\t index);\n>>> +\t\tstreams_.emplace_back(this, stream, streamConfiguration,\n>>> +\t\t\t\t CameraStream::Type::Direct, index);\n>>> \t\tstream->priv = static_cast(&streams_.back());\n>>> \t}\n>>>\n>>> @@ -1272,8 +1272,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>>> \t\t}\n>>>\n>>> \t\tStreamConfiguration &cfg = config_->at(index);\n>>> -\n>>> -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index);\n>>> +\t\tstreams_.emplace_back(this, jpegStream, cfg, type, index);\n>>> \t\tjpegStream->priv = static_cast(&streams_.back());\n>>> \t}\n>>>\n>>> @@ -1405,7 +1404,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n>>> \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n>>>\n>>> \t\t/* Software streams are handled after hardware streams complete. */\n>>> -\t\tif (cameraStream->format() == formats::MJPEG)\n>>> +\t\tif (cameraStream->outputFormat() == HAL_PIXEL_FORMAT_BLOB)\n>>\n>> I understand the premise for this change, it's just so unfortunate that\n>> the android format is so ... well ... unrepresentative :-)\n> \n> More than this, I'm thinking of doing what I'll do with the libcamera\n> StreamConfiguration\n> if (cameraStream->androidStream().format == ... )\n> \n> And not provide a confusing CameraStream::outputFormat() helper at\n> all.\n\nGreat, that looks good!\n\n\n>> I haven't gone to check yet, but I wonder if there is a better boolean\n>> logic to check ...\n>>\n>> \t\tif (format != NV12) perhaps ?\n> \n> We can't bet to have only NV12 and MJPEG\n\nNo, indeed - my premise was that we will (later) need to make the\ndecision somewhere about what a software stream is.\n\nI.e. - if the Android stream is an NV12, and we can only generate a\nYUYV, we will need a software(or opengl :D) format convertor (to support\nUVC).\n\nSo there is going to be a more complex requirement here to decide what a\nsoftware stream is, which will really be\n \"input format != output format\"...\n\nAnyway, this is fine for now.\n\nThe internal buffer handling will in fact make it much easier to look at\nsoftware format conversion anyway.\n\n\n>> I guess it depends on what other combinations we'll see through here.\n>>\n>> But no need to change this now\n>>\n>>> \t\t\tcontinue;\n>>>\n>>> \t\t/*\n>>> @@ -1469,7 +1468,7 @@ void CameraDevice::requestComplete(Request *request)\n>>> \t\tCameraStream *cameraStream =\n>>> \t\t\tstatic_cast(descriptor->buffers[i].stream->priv);\n>>>\n>>> -\t\tif (cameraStream->format() != formats::MJPEG)\n>>> +\t\tif (cameraStream->outputFormat() != HAL_PIXEL_FORMAT_BLOB)\n>>> \t\t\tcontinue;\n>>>\n>>> \t\tEncoder *encoder = cameraStream->encoder();\n>>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n>>> index 52923ec979a7..4e326fa3e1fb 100644\n>>> --- a/src/android/camera_device.h\n>>> +++ b/src/android/camera_device.h\n>>> @@ -43,6 +43,10 @@ public:\n>>> \tunsigned int id() const { return id_; }\n>>> \tcamera3_device_t *camera3Device() { return &camera3Device_; }\n>>> \tconst libcamera::Camera *camera() const { return camera_.get(); }\n>>> +\tlibcamera::CameraConfiguration *cameraConfiguration() const\n>>> +\t{\n>>> +\t\treturn config_.get();\n>>> +\t}\n>>>\n>>> \tint facing() const { return facing_; }\n>>> \tint orientation() const { return orientation_; }\n>>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n>>> index 5b2b625c563b..5c1f1d7da416 100644\n>>> --- a/src/android/camera_stream.cpp\n>>> +++ b/src/android/camera_stream.cpp\n>>> @@ -7,13 +7,23 @@\n>>>\n>>> #include \"camera_stream.h\"\n>>>\n>>> +#include \"camera_device.h\"\n>>> #include \"jpeg/encoder_libjpeg.h\"\n>>>\n>>> using namespace libcamera;\n>>>\n>>> -CameraStream::CameraStream(PixelFormat format, Size size, Type type, unsigned int index)\n>>> -\t: format_(format), size_(size), type_(type), index_(index), encoder(nullptr)\n>>> +CameraStream::CameraStream(CameraDevice *cameraDev,\n>>> +\t\t\t camera3_stream_t *androidStream,\n>>> +\t\t\t const libcamera::StreamConfiguration &cfg,\n>>> +\t\t\t Type type, unsigned int index)\n>>> +\t: cameraDevice_(cameraDev), androidStream_(androidStream), type_(type),\n>>> +\t index_(index), encoder_(nullptr)\n>>> {\n>>> +\tconfig_ = cameraDevice_->cameraConfiguration();\n>>> +\n>>> +\tformat_ = cfg.pixelFormat;\n>>> +\tsize_ = cfg.size;\n>>> +\n>>> \tif (type_ == Type::Internal || type_ == Type::Mapped)\n>>> \t\tencoder_.reset(new EncoderLibJpeg);\n>>> }\n>>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n>>> index d0dc40d81151..2d71a50c78a4 100644\n>>> --- a/src/android/camera_stream.h\n>>> +++ b/src/android/camera_stream.h\n>>> @@ -9,11 +9,16 @@\n>>>\n>>> #include \n>>>\n>>> +#include \n>>> +\n>>> +#include \n>>> #include \n>>> #include \n>>>\n>>> #include \"jpeg/encoder.h\"\n>>>\n>>> +class CameraDevice;\n>>> +\n>>> class CameraStream\n>>> {\n>>> public:\n>>> @@ -99,9 +104,12 @@ public:\n>>> \t\tInternal,\n>>> \t\tMapped,\n>>> \t};\n>>> -\tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n>>> +\tCameraStream(CameraDevice *cameraDev,\n>>\n>> I would have just used 'dev', or 'device', I don't think it conflicts\n>> with anything else in this function does it ?\n>>\n>> or as the class private member it goes into is cameraDevice_, it could\n>> have been cameraDevice ;-) - But it's not important either.\n> \n> I thought about shortening the name too, but as it goes to\n> cameraDevice_ I kept it long.\n> \n> And as Laurent said, if we could unify the naming schemes (ie.\n> everything android prefixed with camera3 and everything libcamera\n> prefixed with.... ) it would be nice\n> \n>>\n>> Reviewed-by: Kieran Bingham \n> \n> Thanks\n> j\n> \n>>\n>>\n>>\n>>> +\t\t camera3_stream_t *androidStream,\n>>> +\t\t const libcamera::StreamConfiguration &cfg,\n>>> \t\t Type type, unsigned int index);\n>>>\n>>> +\tint outputFormat() const { return androidStream_->format; }\n>>> \tconst libcamera::PixelFormat &format() const { return format_; }\n>>> \tconst libcamera::Size &size() const { return size_; }\n>>> \tType type() const { return type_; }\n>>> @@ -111,9 +119,15 @@ public:\n>>> \tint configure(const libcamera::StreamConfiguration &cfg);\n>>>\n>>> private:\n>>> +\tCameraDevice *cameraDevice_;\n>>> +\tlibcamera::CameraConfiguration *config_;\n>>> +\tcamera3_stream_t *androidStream_;\n>>> +\tType type_;\n>>> +\n>>> +\t/* Libcamera facing format and sizes. */\n>>> \tlibcamera::PixelFormat format_;\n>>> \tlibcamera::Size size_;\n>>> -\tType type_;\n>>> +\n>>> \t/*\n>>> \t * The index of the libcamera StreamConfiguration as added during\n>>> \t * configureStreams(). A single libcamera Stream may be used to deliver\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","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 46727BEEDF\n\tfor ;\n\tTue, 6 Oct 2020 12:21:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C063D63C67;\n\tTue, 6 Oct 2020 14:21:04 +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 B56FD60363\n\tfor ;\n\tTue, 6 Oct 2020 14:21:03 +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 1AD8D3B;\n\tTue, 6 Oct 2020 14:21:03 +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=\"tyvordNs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601986863;\n\tbh=bjXM8E9RX/lFfzoLbWIzTfgeh/e7laR86MWLUx20w/8=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=tyvordNsC6un9R6ZvpM689MNyRgk/D6m/feErgbOPfAEkBhQYR7A309Iw1LoIkRgA\n\tUE5pxk7xvZY0X5nr8TbW8pOCTXHuVK1y3Q7VkLxX/NgkeIltoWr7xXGFPcoaJd+/Sj\n\tLIjywNdVEIA8y7wrjXMmMeDa4Vo2w50UJa+mMzkk=","To":"Jacopo Mondi ","References":"<20201005104649.10812-1-laurent.pinchart@ideasonboard.com>\n\t<20201005104649.10812-5-laurent.pinchart@ideasonboard.com>\n\t\n\t<20201006112449.bvjxwkqmz2fnuwkq@uno.localdomain>","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, 6 Oct 2020 13:21:00 +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":"<20201006112449.bvjxwkqmz2fnuwkq@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13024,"web_url":"https://patchwork.libcamera.org/comment/13024/","msgid":"<20201007005758.GB30598@pendragon.ideasonboard.com>","date":"2020-10-07T00:57:58","subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Tue, Oct 06, 2020 at 01:21:00PM +0100, Kieran Bingham wrote:\n> On 06/10/2020 12:24, Jacopo Mondi wrote:\n> > On Mon, Oct 05, 2020 at 05:31:31PM +0100, Kieran Bingham wrote:\n> >> On 05/10/2020 11:46, Laurent Pinchart wrote:\n> >>> From: Jacopo Mondi \n> >>>\n> >>> A CameraStream maps a stream requested by Android to a\n> >>> libcamera::StreamConfiguration. It shall then be constructed with\n> >>> those information clearly specified.\n> >>>> Change the CameraStream constructor to accept an Android\n> >>> camera3_stream_t and a libcamera::StreamConfiguration to copy the\n> >>> format and size information in the class members as no reference can be\n> >>> taken to the StreamConfiguration instances as they're subject to\n> >>> relocations.\n> >>\n> >> I'm not sure we need to say all that, but could have just been \"Pass the\n> >> android camera3_stream_t, and a libcamera::StreamConfiguration to\n> >> identify the source and destination parameters of this stream.\n> >>\n> >>> Pass to the CameraStream constructor a pointer to the CameraDevice class\n> >>> and make the CameraConfiguration accessible. It will be used to retrieve\n> >>> the StreamConfiguration associated with the CameraStream.\n> >>\n> >> Pass a CameraDevice pointer to the CameraStream constructor to allow\n> >> retrieval of the StreamConfiguration associated with the CameraStream.\n> > \n> > Thanks, much better!\n> > \n> >>> Also change the format on which the CameraDevice performs checks to\n> >>> decide if post-processing is required, as the libcamera facing format is\n> >>> not meaningful anymore, but the Android required format should be used\n> >>> instead.\n> >>>\n> >>> Signed-off-by: Jacopo Mondi \n> >>> ---\n> >>> src/android/camera_device.cpp | 11 +++++------\n> >>> src/android/camera_device.h | 4 ++++\n> >>> src/android/camera_stream.cpp | 14 ++++++++++++--\n> >>> src/android/camera_stream.h | 18 ++++++++++++++++--\n> >>> 4 files changed, 37 insertions(+), 10 deletions(-)\n> >>>\n> >>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> >>> index 9c9a5cfa3c2f..2c4dd4dee28c 100644\n> >>> --- a/src/android/camera_device.cpp\n> >>> +++ b/src/android/camera_device.cpp\n> >>> @@ -1216,8 +1216,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >>>\n> >>> \t\tconfig_->addConfiguration(streamConfiguration);\n> >>> \t\tunsigned int index = config_->size() - 1;\n> >>> -\t\tstreams_.emplace_back(format, size, CameraStream::Type::Direct,\n> >>> -\t\t\t\t index);\n> >>> +\t\tstreams_.emplace_back(this, stream, streamConfiguration,\n> >>> +\t\t\t\t CameraStream::Type::Direct, index);\n> >>> \t\tstream->priv = static_cast(&streams_.back());\n> >>> \t}\n> >>>\n> >>> @@ -1272,8 +1272,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >>> \t\t}\n> >>>\n> >>> \t\tStreamConfiguration &cfg = config_->at(index);\n> >>> -\n> >>> -\t\tstreams_.emplace_back(formats::MJPEG, cfg.size, type, index);\n> >>> +\t\tstreams_.emplace_back(this, jpegStream, cfg, type, index);\n> >>> \t\tjpegStream->priv = static_cast(&streams_.back());\n> >>> \t}\n> >>>\n> >>> @@ -1405,7 +1404,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> >>> \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n> >>>\n> >>> \t\t/* Software streams are handled after hardware streams complete. */\n> >>> -\t\tif (cameraStream->format() == formats::MJPEG)\n> >>> +\t\tif (cameraStream->outputFormat() == HAL_PIXEL_FORMAT_BLOB)\n> >>\n> >> I understand the premise for this change, it's just so unfortunate that\n> >> the android format is so ... well ... unrepresentative :-)\n> > \n> > More than this, I'm thinking of doing what I'll do with the libcamera\n> > StreamConfiguration\n> > if (cameraStream->androidStream().format == ... )\n> > \n> > And not provide a confusing CameraStream::outputFormat() helper at\n> > all.\n> \n> Great, that looks good!\n> \n> >> I haven't gone to check yet, but I wonder if there is a better boolean\n> >> logic to check ...\n> >>\n> >> \t\tif (format != NV12) perhaps ?\n> > \n> > We can't bet to have only NV12 and MJPEG\n> \n> No, indeed - my premise was that we will (later) need to make the\n> decision somewhere about what a software stream is.\n> \n> I.e. - if the Android stream is an NV12, and we can only generate a\n> YUYV, we will need a software(or opengl :D) format convertor (to support\n> UVC).\n> \n> So there is going to be a more complex requirement here to decide what a\n> software stream is, which will really be\n\nShould we start calling them post-processed streams (we can bikeshed the\nname), as the post-processing may be handled by hardware (hardware\nscaler or format converter, JPEG encoder, GPU, ...) ?\n\n> \"input format != output format\"...\n> \n> Anyway, this is fine for now.\n> \n> The internal buffer handling will in fact make it much easier to look at\n> software format conversion anyway.\n>\n> >> I guess it depends on what other combinations we'll see through here.\n> >>\n> >> But no need to change this now\n> >>\n> >>> \t\t\tcontinue;\n> >>>\n> >>> \t\t/*\n> >>> @@ -1469,7 +1468,7 @@ void CameraDevice::requestComplete(Request *request)\n> >>> \t\tCameraStream *cameraStream =\n> >>> \t\t\tstatic_cast(descriptor->buffers[i].stream->priv);\n> >>>\n> >>> -\t\tif (cameraStream->format() != formats::MJPEG)\n> >>> +\t\tif (cameraStream->outputFormat() != HAL_PIXEL_FORMAT_BLOB)\n> >>> \t\t\tcontinue;\n> >>>\n> >>> \t\tEncoder *encoder = cameraStream->encoder();\n> >>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> >>> index 52923ec979a7..4e326fa3e1fb 100644\n> >>> --- a/src/android/camera_device.h\n> >>> +++ b/src/android/camera_device.h\n> >>> @@ -43,6 +43,10 @@ public:\n> >>> \tunsigned int id() const { return id_; }\n> >>> \tcamera3_device_t *camera3Device() { return &camera3Device_; }\n> >>> \tconst libcamera::Camera *camera() const { return camera_.get(); }\n> >>> +\tlibcamera::CameraConfiguration *cameraConfiguration() const\n> >>> +\t{\n> >>> +\t\treturn config_.get();\n> >>> +\t}\n> >>>\n> >>> \tint facing() const { return facing_; }\n> >>> \tint orientation() const { return orientation_; }\n> >>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> >>> index 5b2b625c563b..5c1f1d7da416 100644\n> >>> --- a/src/android/camera_stream.cpp\n> >>> +++ b/src/android/camera_stream.cpp\n> >>> @@ -7,13 +7,23 @@\n> >>>\n> >>> #include \"camera_stream.h\"\n> >>>\n> >>> +#include \"camera_device.h\"\n> >>> #include \"jpeg/encoder_libjpeg.h\"\n> >>>\n> >>> using namespace libcamera;\n> >>>\n> >>> -CameraStream::CameraStream(PixelFormat format, Size size, Type type, unsigned int index)\n> >>> -\t: format_(format), size_(size), type_(type), index_(index), encoder(nullptr)\n> >>> +CameraStream::CameraStream(CameraDevice *cameraDev,\n> >>> +\t\t\t camera3_stream_t *androidStream,\n> >>> +\t\t\t const libcamera::StreamConfiguration &cfg,\n> >>> +\t\t\t Type type, unsigned int index)\n> >>> +\t: cameraDevice_(cameraDev), androidStream_(androidStream), type_(type),\n> >>> +\t index_(index), encoder_(nullptr)\n> >>> {\n> >>> +\tconfig_ = cameraDevice_->cameraConfiguration();\n> >>> +\n> >>> +\tformat_ = cfg.pixelFormat;\n> >>> +\tsize_ = cfg.size;\n> >>> +\n> >>> \tif (type_ == Type::Internal || type_ == Type::Mapped)\n> >>> \t\tencoder_.reset(new EncoderLibJpeg);\n> >>> }\n> >>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> >>> index d0dc40d81151..2d71a50c78a4 100644\n> >>> --- a/src/android/camera_stream.h\n> >>> +++ b/src/android/camera_stream.h\n> >>> @@ -9,11 +9,16 @@\n> >>>\n> >>> #include \n> >>>\n> >>> +#include \n> >>> +\n> >>> +#include \n> >>> #include \n> >>> #include \n> >>>\n> >>> #include \"jpeg/encoder.h\"\n> >>>\n> >>> +class CameraDevice;\n> >>> +\n> >>> class CameraStream\n> >>> {\n> >>> public:\n> >>> @@ -99,9 +104,12 @@ public:\n> >>> \t\tInternal,\n> >>> \t\tMapped,\n> >>> \t};\n> >>> -\tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> >>> +\tCameraStream(CameraDevice *cameraDev,\n> >>\n> >> I would have just used 'dev', or 'device', I don't think it conflicts\n> >> with anything else in this function does it ?\n> >>\n> >> or as the class private member it goes into is cameraDevice_, it could\n> >> have been cameraDevice ;-) - But it's not important either.\n> > \n> > I thought about shortening the name too, but as it goes to\n> > cameraDevice_ I kept it long.\n> > \n> > And as Laurent said, if we could unify the naming schemes (ie.\n> > everything android prefixed with camera3 and everything libcamera\n> > prefixed with.... ) it would be nice\n\nWe could also use hal as a prefix for the android side to keep it\nshorter.\n\n> > \n> >> Reviewed-by: Kieran Bingham \n> > \n> > Thanks\n> >\n> >>> +\t\t camera3_stream_t *androidStream,\n> >>> +\t\t const libcamera::StreamConfiguration &cfg,\n> >>> \t\t Type type, unsigned int index);\n> >>>\n> >>> +\tint outputFormat() const { return androidStream_->format; }\n> >>> \tconst libcamera::PixelFormat &format() const { return format_; }\n> >>> \tconst libcamera::Size &size() const { return size_; }\n> >>> \tType type() const { return type_; }\n> >>> @@ -111,9 +119,15 @@ public:\n> >>> \tint configure(const libcamera::StreamConfiguration &cfg);\n> >>>\n> >>> private:\n> >>> +\tCameraDevice *cameraDevice_;\n> >>> +\tlibcamera::CameraConfiguration *config_;\n> >>> +\tcamera3_stream_t *androidStream_;\n> >>> +\tType type_;\n> >>> +\n> >>> +\t/* Libcamera facing format and sizes. */\n> >>> \tlibcamera::PixelFormat format_;\n> >>> \tlibcamera::Size size_;\n> >>> -\tType type_;\n> >>> +\n> >>> \t/*\n> >>> \t * The index of the libcamera StreamConfiguration as added during\n> >>> \t * configureStreams(). A single libcamera Stream may be used to deliver","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 5AA22BEEE0\n\tfor ;\n\tWed, 7 Oct 2020 00:58:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D414363C1B;\n\tWed, 7 Oct 2020 02:58:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BBDA963BD5\n\tfor ;\n\tWed, 7 Oct 2020 02:58:40 +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 299FB1452;\n\tWed, 7 Oct 2020 02:58:40 +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=\"jSpA+xKb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602032320;\n\tbh=YCfw02tqhgY3ANwz4cgL4DelvdJp5aqmuS4/Q+JtIf8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jSpA+xKble3WT7Z7iza02WzJUQJ524HriA4fW/Rx4kTaB8cAq+ll2jiRAWmZzyDHe\n\tr4Tv/eXCr5ek857FvJ3qaboftSobqUY//lo5ncxWcSSC3ZIpTkmIP8HnE6oFLXMCDq\n\th+DdKNUxSpNv10E3I8Q5EoxHfBENJyfDoXoETMuY=","Date":"Wed, 7 Oct 2020 03:57:58 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201007005758.GB30598@pendragon.ideasonboard.com>","References":"<20201005104649.10812-1-laurent.pinchart@ideasonboard.com>\n\t<20201005104649.10812-5-laurent.pinchart@ideasonboard.com>\n\t\n\t<20201006112449.bvjxwkqmz2fnuwkq@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13035,"web_url":"https://patchwork.libcamera.org/comment/13035/","msgid":"<67921dc0-6315-9324-eccc-90605c357427@ideasonboard.com>","date":"2020-10-07T08:47:33","subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 07/10/2020 01:57, Laurent Pinchart wrote:\n> Hello,\n\n:\n\n>>\n>>>> I haven't gone to check yet, but I wonder if there is a better boolean\n>>>> logic to check ...\n>>>>\n>>>> \t\tif (format != NV12) perhaps ?\n>>>\n>>> We can't bet to have only NV12 and MJPEG\n>>\n>> No, indeed - my premise was that we will (later) need to make the\n>> decision somewhere about what a software stream is.\n>>\n>> I.e. - if the Android stream is an NV12, and we can only generate a\n>> YUYV, we will need a software(or opengl :D) format convertor (to support\n>> UVC).\n>>\n>> So there is going to be a more complex requirement here to decide what a\n>> software stream is, which will really be\n> \n> Should we start calling them post-processed streams (we can bikeshed the\n> name), as the post-processing may be handled by hardware (hardware\n> scaler or format converter, JPEG encoder, GPU, ...) ?\n\nYes, indeed - we shouldn't be calling them software streams at all.\n\n\n> \n>> \"input format != output format\"...\n>>\n>> Anyway, this is fine for now.\n>>\n>> The internal buffer handling will in fact make it much easier to look at\n>> software format conversion anyway.\n\n\n\n>>>>> -\tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n>>>>> +\tCameraStream(CameraDevice *cameraDev,\n>>>>\n>>>> I would have just used 'dev', or 'device', I don't think it conflicts\n>>>> with anything else in this function does it ?\n>>>>\n>>>> or as the class private member it goes into is cameraDevice_, it could\n>>>> have been cameraDevice ;-) - But it's not important either.\n>>>\n>>> I thought about shortening the name too, but as it goes to\n>>> cameraDevice_ I kept it long.\n>>>\n>>> And as Laurent said, if we could unify the naming schemes (ie.\n>>> everything android prefixed with camera3 and everything libcamera\n>>> prefixed with.... ) it would be nice\n> \n> We could also use hal as a prefix for the android side to keep it\n> shorter.\n\nAnd keep libcamera objects without a prefix? Or lc ?\n\n> \n>>>\n>>>> Reviewed-by: Kieran Bingham \n>>>\n>>> Thanks\n>>>\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 7B1EDBEEE0\n\tfor ;\n\tWed, 7 Oct 2020 08:47:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F330063CAF;\n\tWed, 7 Oct 2020 10:47:37 +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 453F16035F\n\tfor ;\n\tWed, 7 Oct 2020 10:47:36 +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 A36609DA;\n\tWed, 7 Oct 2020 10:47:35 +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=\"dQSwV+Le\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602060455;\n\tbh=aC1XF30261d50BcV09AzJcoL9pL7ET6e4trH2c9B5uI=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=dQSwV+LeUJezgOW+m3ACrZd/bAxSLL5+Ih96yLRQqvTpReHBwMDAaLDgYoB7w0e9g\n\tD/Bp3yoQxjrrNuJsiCkNWWipBllGxtSrlbBVMlMcbrYTND/+3ERvjLDtk01gU51kDh\n\t2XgMlUAWTzd4Yc1kBwmZfEo+o8ktxmx8TOFB4a6Q=","To":"Laurent Pinchart ","References":"<20201005104649.10812-1-laurent.pinchart@ideasonboard.com>\n\t<20201005104649.10812-5-laurent.pinchart@ideasonboard.com>\n\t\n\t<20201006112449.bvjxwkqmz2fnuwkq@uno.localdomain>\n\t\n\t<20201007005758.GB30598@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":"<67921dc0-6315-9324-eccc-90605c357427@ideasonboard.com>","Date":"Wed, 7 Oct 2020 09:47:33 +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":"<20201007005758.GB30598@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13074,"web_url":"https://patchwork.libcamera.org/comment/13074/","msgid":"<20201007133738.GL3937@pendragon.ideasonboard.com>","date":"2020-10-07T13:37:38","subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Oct 07, 2020 at 09:47:33AM +0100, Kieran Bingham wrote:\n> On 07/10/2020 01:57, Laurent Pinchart wrote:\n> > Hello,\n> \n> :\n> \n> >>>> I haven't gone to check yet, but I wonder if there is a better boolean\n> >>>> logic to check ...\n> >>>>\n> >>>> \t\tif (format != NV12) perhaps ?\n> >>>\n> >>> We can't bet to have only NV12 and MJPEG\n> >>\n> >> No, indeed - my premise was that we will (later) need to make the\n> >> decision somewhere about what a software stream is.\n> >>\n> >> I.e. - if the Android stream is an NV12, and we can only generate a\n> >> YUYV, we will need a software(or opengl :D) format convertor (to support\n> >> UVC).\n> >>\n> >> So there is going to be a more complex requirement here to decide what a\n> >> software stream is, which will really be\n> > \n> > Should we start calling them post-processed streams (we can bikeshed the\n> > name), as the post-processing may be handled by hardware (hardware\n> > scaler or format converter, JPEG encoder, GPU, ...) ?\n> \n> Yes, indeed - we shouldn't be calling them software streams at all.\n> \n> >> \"input format != output format\"...\n> >>\n> >> Anyway, this is fine for now.\n> >>\n> >> The internal buffer handling will in fact make it much easier to look at\n> >> software format conversion anyway.\n> \n> \n> \n> >>>>> -\tCameraStream(libcamera::PixelFormat format, libcamera::Size size,\n> >>>>> +\tCameraStream(CameraDevice *cameraDev,\n> >>>>\n> >>>> I would have just used 'dev', or 'device', I don't think it conflicts\n> >>>> with anything else in this function does it ?\n> >>>>\n> >>>> or as the class private member it goes into is cameraDevice_, it could\n> >>>> have been cameraDevice ;-) - But it's not important either.\n> >>>\n> >>> I thought about shortening the name too, but as it goes to\n> >>> cameraDevice_ I kept it long.\n> >>>\n> >>> And as Laurent said, if we could unify the naming schemes (ie.\n> >>> everything android prefixed with camera3 and everything libcamera\n> >>> prefixed with.... ) it would be nice\n> > \n> > We could also use hal as a prefix for the android side to keep it\n> > shorter.\n> \n> And keep libcamera objects without a prefix? Or lc ?\n\nI'd go for no prefix, lc sounds really weird (but maybe I'd get use to\nit).\n\n> >>>> Reviewed-by: Kieran Bingham ","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 B7987BEEE0\n\tfor ;\n\tWed, 7 Oct 2020 13:38:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 87895605CF;\n\tWed, 7 Oct 2020 15:38:23 +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 2485F605C1\n\tfor ;\n\tWed, 7 Oct 2020 15:38:22 +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 737759DA;\n\tWed, 7 Oct 2020 15:38:20 +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=\"bqL6XYF4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602077900;\n\tbh=i4xahxhmDuJg0hVVtmVu/kJ5Z+k5T0waG2AeICEMDuI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bqL6XYF4Ox+T6ARa7L4CG8qN9EpOQDHrl4D5vNTs0gz1Wo3BwvxBOfcyd+BJpg5Jq\n\tHpaYi349uKWEDhvy1diZ6jFwskOpOJhZDCgON/HmDOUUGfymBw6B9FYIZ2F48Qs/6y\n\tOiDlJAi8BTvFMnBHVdhaG8XNbInsvX8Tc6v2albc=","Date":"Wed, 7 Oct 2020 16:37:38 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201007133738.GL3937@pendragon.ideasonboard.com>","References":"<20201005104649.10812-1-laurent.pinchart@ideasonboard.com>\n\t<20201005104649.10812-5-laurent.pinchart@ideasonboard.com>\n\t\n\t<20201006112449.bvjxwkqmz2fnuwkq@uno.localdomain>\n\t\n\t<20201007005758.GB30598@pendragon.ideasonboard.com>\n\t<67921dc0-6315-9324-eccc-90605c357427@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<67921dc0-6315-9324-eccc-90605c357427@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 04/15] android: camera_stream:\n\tConstruct with Android stream","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]