[{"id":12319,"web_url":"https://patchwork.libcamera.org/comment/12319/","msgid":"<20200905213739.GJ6319@pendragon.ideasonboard.com>","date":"2020-09-05T21:37:39","subject":"Re: [libcamera-devel] [PATCH v2 07/12] android: camera_device: Get\n\trid of stream counter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Sep 02, 2020 at 05:22:31PM +0200, Jacopo Mondi wrote:\n> Now that CameraConfiguration::addConfiguration() returns the index\n> of the newly added configuration, there's no need to use a custom\n> counter to keep track of the StreamConfiguration index associated with\n> a CameraStream.\n> \n> Reviewed-by: Kieran Bingham \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 13 ++-----------\n> 1 file changed, 2 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b110bfe43ece..ace7f6b17b4a 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1195,12 +1195,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \tstreams_.clear();\n> \tstreams_.reserve(stream_list->num_streams);\n> \n> -\t/*\n> -\t * Track actually created streams, as there may not be a 1:1 mapping of\n> -\t * camera3 streams to libcamera streams.\n> -\t */\n> -\tunsigned int streamIndex = 0;\n> -\n> \t/* First handle all non-MJPEG streams. */\n> \tfor (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n> \t\tcamera3_stream_t *stream = stream_list->streams[i];\n> @@ -1230,9 +1224,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\tstreamConfiguration.size = size;\n> \t\tstreamConfiguration.pixelFormat = format;\n> \n> -\t\tconfig_->addConfiguration(streamConfiguration);\n> -\n> -\t\tstreams_[i].index = streamIndex++;\n> +\t\tstreams_[i].index = config_->addConfiguration(streamConfiguration);\n\nAs explained in the review of 06/12, this could also be\n\n\t\tconfig_->addConfiguration(streamConfiguration);\n\t\tstreams_[i].index = config_->size() - 1;\n\nIn any case,\n\nAcked-by: Laurent Pinchart \n\n> \t}\n> \n> \t/* Now handle MJPEG streams, adding a new stream if required. */\n> @@ -1280,8 +1272,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t\tLOG(HAL, Info) << \"Adding \" << streamConfiguration.toString()\n> \t\t\t\t << \" for MJPEG support\";\n> \n> -\t\t\tconfig_->addConfiguration(streamConfiguration);\n> -\t\t\tstreams_[i].index = streamIndex++;\n> +\t\t\tstreams_[i].index = config_->addConfiguration(streamConfiguration);\n> \t\t}\n> \t}\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 07EB4BDB1C\n\tfor ;\n\tSat, 5 Sep 2020 21:38:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 936A462901;\n\tSat, 5 Sep 2020 23:38:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 249C562901\n\tfor ;\n\tSat, 5 Sep 2020 23:38:03 +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 7C9B7335;\n\tSat, 5 Sep 2020 23:38:02 +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=\"EM/1/08f\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599341882;\n\tbh=1Y+83jsByIr6BSjx89FCgM9M1zLaaFtO/VzhKsYOyQ8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EM/1/08f6mck2q80qOCdLlKYJW1NjgMYhuNVV/NfSWoQpPfq3OAzqj5Oetf6dAJiB\n\tUPHWHheR+7XSjyXoaFyE2yczhVcr06Zeq55M4/kPVH6Q1T1GQyyUwFNd8CEfheWwYz\n\t1XmzoGYNRZARp2Og2j8jL6OUAtEg5SR1G9sV2fZU=","Date":"Sun, 6 Sep 2020 00:37:39 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200905213739.GJ6319@pendragon.ideasonboard.com>","References":"<20200902152236.69770-1-jacopo@jmondi.org>\n\t<20200902152236.69770-8-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200902152236.69770-8-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 07/12] android: camera_device: Get\n\trid of stream counter","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":"tfiga@google.com, libcamera-devel@lists.libcamera.org, hiroh@google.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12362,"web_url":"https://patchwork.libcamera.org/comment/12362/","msgid":"","date":"2020-09-08T04:07:12","subject":"Re: [libcamera-devel] [PATCH v2 07/12] android: camera_device: Get\n\trid of stream counter","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"I personally prefer the way Laurent suggested.\n\nOn Sun, Sep 6, 2020 at 6:38 AM Laurent Pinchart\n wrote:\n>\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Sep 02, 2020 at 05:22:31PM +0200, Jacopo Mondi wrote:\n> > Now that CameraConfiguration::addConfiguration() returns the index\n> > of the newly added configuration, there's no need to use a custom\n> > counter to keep track of the StreamConfiguration index associated with\n> > a CameraStream.\n> >\n> > Reviewed-by: Kieran Bingham \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 13 ++-----------\n> > 1 file changed, 2 insertions(+), 11 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index b110bfe43ece..ace7f6b17b4a 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1195,12 +1195,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > streams_.clear();\n> > streams_.reserve(stream_list->num_streams);\n> >\n> > - /*\n> > - * Track actually created streams, as there may not be a 1:1 mapping of\n> > - * camera3 streams to libcamera streams.\n> > - */\n> > - unsigned int streamIndex = 0;\n> > -\n> > /* First handle all non-MJPEG streams. */\n> > for (unsigned int i = 0; i < stream_list->num_streams; ++i) {\n> > camera3_stream_t *stream = stream_list->streams[i];\n> > @@ -1230,9 +1224,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > streamConfiguration.size = size;\n> > streamConfiguration.pixelFormat = format;\n> >\n> > - config_->addConfiguration(streamConfiguration);\n> > -\n> > - streams_[i].index = streamIndex++;\n> > + streams_[i].index = config_->addConfiguration(streamConfiguration);\n>\n> As explained in the review of 06/12, this could also be\n>\n> config_->addConfiguration(streamConfiguration);\n> streams_[i].index = config_->size() - 1;\n>\n> In any case,\n>\n> Acked-by: Laurent Pinchart \n>\n> > }\n> >\n> > /* Now handle MJPEG streams, adding a new stream if required. */\n> > @@ -1280,8 +1272,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> > LOG(HAL, Info) << \"Adding \" << streamConfiguration.toString()\n> > << \" for MJPEG support\";\n> >\n> > - config_->addConfiguration(streamConfiguration);\n> > - streams_[i].index = streamIndex++;\n> > + streams_[i].index = config_->addConfiguration(streamConfiguration);\n> > }\n> > }\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 41CADBDB1D\n\tfor ;\n\tTue, 8 Sep 2020 04:07:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9DDC62BAA;\n\tTue, 8 Sep 2020 06:07:23 +0200 (CEST)","from mail-ed1-x543.google.com (mail-ed1-x543.google.com\n\t[IPv6:2a00:1450:4864:20::543])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0206E603EE\n\tfor ;\n\tTue, 8 Sep 2020 06:07:22 +0200 (CEST)","by mail-ed1-x543.google.com with SMTP id i1so14389018edv.2\n\tfor ;\n\tMon, 07 Sep 2020 21:07:22 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"ir1C4rED\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=62JvpB+1uSKpWgF7kqGOy4rNfVpaAD0wAKsG065txj0=;\n\tb=ir1C4rEDuPoVi4BjQ/4PjZwOhVJKUWSqbLl/jgqCDuJEu7TncY+ZGKmSAlNGFgRWLE\n\txtv5hFLody9NWd90/sECDiY1C2i8fYck4QFzpdlF3kF7JKtO/ueErpFVimzVFl4djI6f\n\tvBAaKv5zrhSA6FbjJIJn9M5Cd8XXq2hs4ZrJY=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=62JvpB+1uSKpWgF7kqGOy4rNfVpaAD0wAKsG065txj0=;\n\tb=XoEuSS9nO5oYGN4gGtG3BIhVeYQRem5swMTc33KQTcvve61CSqlYpZRWYwltvxZnBV\n\ttb5BaXZSfk4Aj9+L7xa1uVg8t//8bKFGP1H+nI1Sbgc5mhKIx3yxmtbpmU1qVUjEMih0\n\td8a+YpOq3X3hKi45mjr0TPTYKUcozaDHIKjTfXGEIeOG87JCPDpP8NZGkMyXUzMfe7eE\n\tEpcyC7QxMkqnQpDfMxCFMkqGqo7fkIdgju7cVV+RgMeCvSPoT56VfApbyXJ2uFawmCeM\n\tjSVOjprDtYScqNO0eQWx5ieNcuC/cgVUqOv/cVu5++0vDtPo33vbKdsTg6fM8w43HI1K\n\tkM6Q==","X-Gm-Message-State":"AOAM531fSI4G9b8gda5P+Q/kquDHM5fjRhx59tIEhEtzucoDaZWtIrmD\n\tYcPIJ/UGn7oUR99IPu14R6pQ9nbShwJ4ZmkyGL/tPg==","X-Google-Smtp-Source":"ABdhPJzZM+ovw7JqWH1wNA0kuBNWeL7tY4ZFCPwJW+8WyScZPW96GrnUENWUNPEmJOFZXX9qDmJGHalmtZ4BJDah0iA=","X-Received":"by 2002:a05:6402:70f:: with SMTP id\n\tw15mr25454581edx.202.1599538042669; \n\tMon, 07 Sep 2020 21:07:22 -0700 (PDT)","MIME-Version":"1.0","References":"<20200902152236.69770-1-jacopo@jmondi.org>\n\t<20200902152236.69770-8-jacopo@jmondi.org>\n\t<20200905213739.GJ6319@pendragon.ideasonboard.com>","In-Reply-To":"<20200905213739.GJ6319@pendragon.ideasonboard.com>","From":"Hirokazu Honda ","Date":"Tue, 8 Sep 2020 13:07:12 +0900","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v2 07/12] android: camera_device: Get\n\trid of stream counter","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":"Tomasz Figa , libcamera-devel@lists.libcamera.org,\n\tHirokazu Honda ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]