[{"id":19803,"web_url":"https://patchwork.libcamera.org/comment/19803/","msgid":"<20210923074044.GP4382@pyrite.rasen.tech>","date":"2021-09-23T07:40:44","subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Thu, Sep 23, 2021 at 12:54:53PM +0530, Umang Jain wrote:\n> Generation of thumbnail is not occuring currently because\n> ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed\n> to PostProcessorJpeg::process(). The commit 1264628d3c92(\"android:\n> jpeg: Configure thumbnailer based on request metadata\") introduced\n> the mechanism to retrieve the thumbanil size from request metadata,\n> however it didn't add the counterpart i.e. inserting the size in\n> the request metadata in request metadata template, at the first place.\n> \n> The patch fixes this issue by setting ANDROID_JPEG_THUMBNAIL_SIZE in\n> the request metadata template populated by\n> CameraCapabilities::requestTemplatePreview(). The value for\n> ANDROID_JPEG_THUMBNAIL_SIZE is set to be the first non-zero size\n> reported by static metadata ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES.\n> \n> Fixes: 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request metadata\")\n> Signed-off-by: Umang Jain \n> Reviewed-by: Laurent Pinchart \n> Reviewed-by: Hirokazu Honda \n> ---\n> src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++-\n> 1 file changed, 23 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 238b44db..ba6adf73 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -1347,7 +1347,7 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> \t * CameraMetadata is capable to resize the container on the fly, if the\n> \t * number of entries get exceeded.\n> \t */\n> -\tauto requestTemplate = std::make_unique(21, 36);\n> +\tauto requestTemplate = std::make_unique(22, 38);\n> \tif (!requestTemplate->isValid()) {\n> \t\treturn nullptr;\n> \t}\n> @@ -1368,6 +1368,28 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n> \t\t\t\t entry.data.i32, 2);\n> \n> +\t/*\n> +\t * Get thumbnail sizes from static metadata and add the first non-zero\n> +\t * size to the template.\n> +\t */\n> +\tfound = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n> +\t\t\t\t\t &entry);\n> +\tASSERT(found && entry.count >= 4);\n> +\tunsigned int j = 0;\n> +\twhile (j < entry.count / 2) {\n\nSince you're incrementing j by 2, shouldn't the limit be entry.count?\n\n> +\t\tif (entry.data.i32[j] == 0 || entry.data.i32[j + 1] == 0) {\n> +\t\t\tj += 2;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> +\t\t\t\t\t entry.data.i32 + j, 2);\n> +\t\tbreak;\n> +\t}\n> +\n> +\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> +\t\t\t\t entry.data.i32 + 2, 2);\n\niirc adding an entry that already exists causes weird problems...\n\n\nPaul\n\n> +\n> \tuint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n> \n> -- \n> 2.31.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 5354CBDC71\n\tfor ;\n\tThu, 23 Sep 2021 07:40:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A18606918C;\n\tThu, 23 Sep 2021 09:40:52 +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 00C95687DC\n\tfor ;\n\tThu, 23 Sep 2021 09:40:51 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 65F8545E;\n\tThu, 23 Sep 2021 09:40:50 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"R3d6Ju9f\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632382851;\n\tbh=h8zI4qy2xFNYl1OSB0CUgqtOqUrI/oxI0KWbw5eqoP0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=R3d6Ju9f72RMSJNVF8kUNtBW8q8/JpCF6/adMomQ8GGrUzy7S/IGmeJjF5A/+xf/Z\n\tJJbhkmY0DE0K3RTsIEIviQBH6q8DKckUTfPa6NDs5+K9Bner4Xyr/4ncJYTKWNIk3N\n\t5aiNYoe2Smog6YgBaB+ODTtlCdmkoqJvarQculsw=","Date":"Thu, 23 Sep 2021 16:40:44 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain ","Message-ID":"<20210923074044.GP4382@pyrite.rasen.tech>","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210923072453.130346-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19804,"web_url":"https://patchwork.libcamera.org/comment/19804/","msgid":"<09fafc15-3475-8aa4-c3aa-c12a8c5aa986@ideasonboard.com>","date":"2021-09-23T07:53:53","subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nOn 9/23/21 1:10 PM, paul.elder@ideasonboard.com wrote:\n> Hi Umang,\n>\n> On Thu, Sep 23, 2021 at 12:54:53PM +0530, Umang Jain wrote:\n>> Generation of thumbnail is not occuring currently because\n>> ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed\n>> to PostProcessorJpeg::process(). The commit 1264628d3c92(\"android:\n>> jpeg: Configure thumbnailer based on request metadata\") introduced\n>> the mechanism to retrieve the thumbanil size from request metadata,\n>> however it didn't add the counterpart i.e. inserting the size in\n>> the request metadata in request metadata template, at the first place.\n>>\n>> The patch fixes this issue by setting ANDROID_JPEG_THUMBNAIL_SIZE in\n>> the request metadata template populated by\n>> CameraCapabilities::requestTemplatePreview(). The value for\n>> ANDROID_JPEG_THUMBNAIL_SIZE is set to be the first non-zero size\n>> reported by static metadata ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES.\n>>\n>> Fixes: 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request metadata\")\n>> Signed-off-by: Umang Jain \n>> Reviewed-by: Laurent Pinchart \n>> Reviewed-by: Hirokazu Honda \n>> ---\n>> src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++-\n>> 1 file changed, 23 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n>> index 238b44db..ba6adf73 100644\n>> --- a/src/android/camera_capabilities.cpp\n>> +++ b/src/android/camera_capabilities.cpp\n>> @@ -1347,7 +1347,7 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n>> \t * CameraMetadata is capable to resize the container on the fly, if the\n>> \t * number of entries get exceeded.\n>> \t */\n>> -\tauto requestTemplate = std::make_unique(21, 36);\n>> +\tauto requestTemplate = std::make_unique(22, 38);\n>> \tif (!requestTemplate->isValid()) {\n>> \t\treturn nullptr;\n>> \t}\n>> @@ -1368,6 +1368,28 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n>> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n>> \t\t\t\t entry.data.i32, 2);\n>> \n>> +\t/*\n>> +\t * Get thumbnail sizes from static metadata and add the first non-zero\n>> +\t * size to the template.\n>> +\t */\n>> +\tfound = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n>> +\t\t\t\t\t &entry);\n>> +\tASSERT(found && entry.count >= 4);\n>> +\tunsigned int j = 0;\n>> +\twhile (j < entry.count / 2) {\n> Since you're incrementing j by 2, shouldn't the limit be entry.count?\n\n\nMy understanding is 2 entries make 1 size, no? \nANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES be like for e.g.\n\n     {0, 0, w1, h1, w2, h2}\n\nSo entry.count is 6 but sizes are 3\n\nTo go to next Size, incremement by 2 so, j += 2\n\nDoes it make sense?\n\n\n>\n>> +\t\tif (entry.data.i32[j] == 0 || entry.data.i32[j + 1] == 0) {\n>> +\t\t\tj += 2;\n>> +\t\t\tcontinue;\n>> +\t\t}\n>> +\n>> +\t\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>> +\t\t\t\t\t entry.data.i32 + j, 2);\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>> +\t\t\t\t entry.data.i32 + 2, 2);\n> iirc adding an entry that already exists causes weird problems...\n\n\nAh crap, this should be outside the loop. I am re-assigning it here. \nSorry, rebase fiasco it seems.\n\n>\n>\n> Paul\n>\n>> +\n>> \tuint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n>> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n>> \n>> -- \n>> 2.31.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 CFC78BF01C\n\tfor ;\n\tThu, 23 Sep 2021 07:53:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 439A46918C;\n\tThu, 23 Sep 2021 09:53:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 12BC5687DC\n\tfor ;\n\tThu, 23 Sep 2021 09:53:58 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.124])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 30A6B45E;\n\tThu, 23 Sep 2021 09:53:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NSN9Dncr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632383637;\n\tbh=AX+R8pNnfaSMHYHlMjHDzNSA8NRL3vlvFoW/UfHDavs=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=NSN9Dncrdbl8XU7vZNpRuY75IA0HAXk2/AjkgjZ6XUo63ruFA6FWw6GxvKUSBGzJp\n\tArVMnfkKlOYFgJKA1enCFX4yLrkG/GTr4LVtVrY6h4WO6HMGIJ5LReRLxI9S3nbeFd\n\tfgGQAeU11iKRdKUXCAu4wjikWcKQxf61Kj+FvpEo=","To":"paul.elder@ideasonboard.com","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-3-umang.jain@ideasonboard.com>\n\t<20210923074044.GP4382@pyrite.rasen.tech>","From":"Umang Jain ","Message-ID":"<09fafc15-3475-8aa4-c3aa-c12a8c5aa986@ideasonboard.com>","Date":"Thu, 23 Sep 2021 13:23:53 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210923074044.GP4382@pyrite.rasen.tech>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19805,"web_url":"https://patchwork.libcamera.org/comment/19805/","msgid":"<0d52ce04-641b-851b-0eb1-709c370fca2b@ideasonboard.com>","date":"2021-09-23T08:03:31","subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi again,\n\nOn 9/23/21 1:23 PM, Umang Jain wrote:\n> Hi Paul,\n>\n> On 9/23/21 1:10 PM, paul.elder@ideasonboard.com wrote:\n>> Hi Umang,\n>>\n>> On Thu, Sep 23, 2021 at 12:54:53PM +0530, Umang Jain wrote:\n>>> Generation of thumbnail is not occuring currently because\n>>> ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed\n>>> to PostProcessorJpeg::process(). The commit 1264628d3c92(\"android:\n>>> jpeg: Configure thumbnailer based on request metadata\") introduced\n>>> the mechanism to retrieve the thumbanil size from request metadata,\n>>> however it didn't add the counterpart i.e. inserting the size in\n>>> the request metadata in request metadata template, at the first place.\n>>>\n>>> The patch fixes this issue by setting ANDROID_JPEG_THUMBNAIL_SIZE in\n>>> the request metadata template populated by\n>>> CameraCapabilities::requestTemplatePreview(). The value for\n>>> ANDROID_JPEG_THUMBNAIL_SIZE is set to be the first non-zero size\n>>> reported by static metadata ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES.\n>>>\n>>> Fixes: 1264628d3c92(\"android: jpeg: Configure thumbnailer based on \n>>> request metadata\")\n>>> Signed-off-by: Umang Jain \n>>> Reviewed-by: Laurent Pinchart \n>>> Reviewed-by: Hirokazu Honda \n>>> ---\n>>>   src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++-\n>>>   1 file changed, 23 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/android/camera_capabilities.cpp \n>>> b/src/android/camera_capabilities.cpp\n>>> index 238b44db..ba6adf73 100644\n>>> --- a/src/android/camera_capabilities.cpp\n>>> +++ b/src/android/camera_capabilities.cpp\n>>> @@ -1347,7 +1347,7 @@ std::unique_ptr \n>>> CameraCapabilities::requestTemplatePreview() con\n>>>        * CameraMetadata is capable to resize the container on the \n>>> fly, if the\n>>>        * number of entries get exceeded.\n>>>        */\n>>> -    auto requestTemplate = std::make_unique(21, 36);\n>>> +    auto requestTemplate = std::make_unique(22, 38);\n>>>       if (!requestTemplate->isValid()) {\n>>>           return nullptr;\n>>>       }\n>>> @@ -1368,6 +1368,28 @@ std::unique_ptr \n>>> CameraCapabilities::requestTemplatePreview() con\n>>> requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n>>>                     entry.data.i32, 2);\n>>>   +    /*\n>>> +     * Get thumbnail sizes from static metadata and add the first \n>>> non-zero\n>>> +     * size to the template.\n>>> +     */\n>>> +    found = \n>>> staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n>>> +                      &entry);\n>>> +    ASSERT(found && entry.count >= 4);\n>>> +    unsigned int j = 0;\n>>> +    while (j < entry.count / 2) {\n>> Since you're incrementing j by 2, shouldn't the limit be entry.count?\n>\n>\n> My understanding is 2 entries make 1 size, no? \n> ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES be like for e.g.\n>\n>     {0, 0, w1, h1, w2, h2}\n>\n> So entry.count is 6 but sizes are 3\n>\n> To go to next Size, incremement by 2 so, j += 2\n>\n> Does it make sense?\n>\n>\n>>\n>>> +        if (entry.data.i32[j] == 0 || entry.data.i32[j + 1] == 0) {\n>>> +            j += 2;\n>>> +            continue;\n>>> +        }\n>>> +\n>>> + requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>>> +                      entry.data.i32 + j, 2);\n>>> +        break;\n>>> +    }\n>>> +\n>>> +    requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>>> +                  entry.data.i32 + 2, 2);\n>> iirc adding an entry that already exists causes weird problems...\n>\n>\n> Ah crap, this should be outside the loop. I am re-assigning it here. \n> Sorry, rebase fiasco it seems.\n\n\nerr,  I meant:\n\nThis should _not_ be outside the loop.\n\n>\n>>\n>>\n>> Paul\n>>\n>>> +\n>>>       uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n>>>       requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n>>>   --\n>>> 2.31.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 3D60ABDC71\n\tfor ;\n\tThu, 23 Sep 2021 08:03:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82E416918E;\n\tThu, 23 Sep 2021 10:03:39 +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 4EE0F687DC\n\tfor ;\n\tThu, 23 Sep 2021 10:03:38 +0200 (CEST)","from [IPv6:2405:204:8203:58de:7651:79c1:5e62:dca9] (unknown\n\t[IPv6:2405:204:8203:58de:7651:79c1:5e62:dca9])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A377D45E;\n\tThu, 23 Sep 2021 10:03:36 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"s4babhmV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632384217;\n\tbh=7GlWFiy6Y1/TmfFpiZ1AH6b3mkroZ3TPPRmLruykfSE=;\n\th=Subject:From:To:Cc:References:Date:In-Reply-To:From;\n\tb=s4babhmVe8YH0GBjQTskq8P/Wt/95AH7BQJOSfRhkxGBqcHoUGdrEIIe3vjxcFcVV\n\t/+nwmyIwnJ7B6YSCn5ZuNIblOpr60ksI4EG3K9f3cBzTpdJbrYq4t1eXvWa0GqwfJe\n\tzjaGsmiBQKqbKhnFRPFWsoLhTIxhATXtray+Yho4=","From":"Umang Jain ","To":"paul.elder@ideasonboard.com","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-3-umang.jain@ideasonboard.com>\n\t<20210923074044.GP4382@pyrite.rasen.tech>\n\t<09fafc15-3475-8aa4-c3aa-c12a8c5aa986@ideasonboard.com>","Message-ID":"<0d52ce04-641b-851b-0eb1-709c370fca2b@ideasonboard.com>","Date":"Thu, 23 Sep 2021 13:33:31 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<09fafc15-3475-8aa4-c3aa-c12a8c5aa986@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19806,"web_url":"https://patchwork.libcamera.org/comment/19806/","msgid":"<20210923095901.GQ4382@pyrite.rasen.tech>","date":"2021-09-23T09:59:02","subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Thu, Sep 23, 2021 at 01:23:53PM +0530, Umang Jain wrote:\n> Hi Paul,\n> \n> On 9/23/21 1:10 PM, paul.elder@ideasonboard.com wrote:\n> > Hi Umang,\n> > \n> > On Thu, Sep 23, 2021 at 12:54:53PM +0530, Umang Jain wrote:\n> > > Generation of thumbnail is not occuring currently because\n> > > ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed\n> > > to PostProcessorJpeg::process(). The commit 1264628d3c92(\"android:\n> > > jpeg: Configure thumbnailer based on request metadata\") introduced\n> > > the mechanism to retrieve the thumbanil size from request metadata,\n> > > however it didn't add the counterpart i.e. inserting the size in\n> > > the request metadata in request metadata template, at the first place.\n> > > \n> > > The patch fixes this issue by setting ANDROID_JPEG_THUMBNAIL_SIZE in\n> > > the request metadata template populated by\n> > > CameraCapabilities::requestTemplatePreview(). The value for\n> > > ANDROID_JPEG_THUMBNAIL_SIZE is set to be the first non-zero size\n> > > reported by static metadata ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES.\n> > > \n> > > Fixes: 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request metadata\")\n> > > Signed-off-by: Umang Jain \n> > > Reviewed-by: Laurent Pinchart \n> > > Reviewed-by: Hirokazu Honda \n> > > ---\n> > > src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++-\n> > > 1 file changed, 23 insertions(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > index 238b44db..ba6adf73 100644\n> > > --- a/src/android/camera_capabilities.cpp\n> > > +++ b/src/android/camera_capabilities.cpp\n> > > @@ -1347,7 +1347,7 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> > > \t * CameraMetadata is capable to resize the container on the fly, if the\n> > > \t * number of entries get exceeded.\n> > > \t */\n> > > -\tauto requestTemplate = std::make_unique(21, 36);\n> > > +\tauto requestTemplate = std::make_unique(22, 38);\n> > > \tif (!requestTemplate->isValid()) {\n> > > \t\treturn nullptr;\n> > > \t}\n> > > @@ -1368,6 +1368,28 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> > > \trequestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n> > > \t\t\t\t entry.data.i32, 2);\n> > > +\t/*\n> > > +\t * Get thumbnail sizes from static metadata and add the first non-zero\n> > > +\t * size to the template.\n> > > +\t */\n> > > +\tfound = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n> > > +\t\t\t\t\t &entry);\n> > > +\tASSERT(found && entry.count >= 4);\n> > > +\tunsigned int j = 0;\n> > > +\twhile (j < entry.count / 2) {\n> > Since you're incrementing j by 2, shouldn't the limit be entry.count?\n> \n> \n> My understanding is 2 entries make 1 size, no?\n> ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES be like for e.g.\n> \n>     {0, 0, w1, h1, w2, h2}\n> \n> So entry.count is 6 but sizes are 3\n> \n> To go to next Size, incremement by 2 so, j += 2\n\nThat's true, but for example:\n\n{0, 0, w1, h1, w2, h2}\n\nIteration 1: j = 0, so you take [0, 1]; afterward j += 2\nIteration 2: j = 2, so you take [2, 3]; afterward j += 2\nIteration 3: j = 4, so you take [4, 5]; afterward j += 2\nItreation 4: j = 6, fails j < entry.count; skip\n\nIf you stop at entry.count / 2 then you'll skip the second half of\npairs.\n\n\nPaul\n\n> \n> Does it make sense?\n> \n> \n> > \n> > > +\t\tif (entry.data.i32[j] == 0 || entry.data.i32[j + 1] == 0) {\n> > > +\t\t\tj += 2;\n> > > +\t\t\tcontinue;\n> > > +\t\t}\n> > > +\n> > > +\t\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> > > +\t\t\t\t\t entry.data.i32 + j, 2);\n> > > +\t\tbreak;\n> > > +\t}\n> > > +\n> > > +\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> > > +\t\t\t\t entry.data.i32 + 2, 2);\n> > iirc adding an entry that already exists causes weird problems...\n> \n> \n> Ah crap, this should be outside the loop. I am re-assigning it here. Sorry,\n> rebase fiasco it seems.\n> \n> > \n> > \n> > Paul\n> > \n> > > +\n> > > \tuint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n> > > \trequestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n> > > -- \n> > > 2.31.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 CF302BF01C\n\tfor ;\n\tThu, 23 Sep 2021 09:59:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B1A56918A;\n\tThu, 23 Sep 2021 11:59:12 +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 3EB9B687DC\n\tfor ;\n\tThu, 23 Sep 2021 11:59:10 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A373845E;\n\tThu, 23 Sep 2021 11:59:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MF2HayU5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632391149;\n\tbh=MYoeXtc0bS6knJN/01cYx6k3tPsd5nYv3m893kz9QHE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MF2HayU5Cgm7amjxOG8SDRNkjo+LkojEUL263P+dNMo5bQNqj0VfNXep0h4pc811D\n\tWyAdo22ySM1eSxu9r8dDzPg31NBwHERAHlDl3G0vKQmpNI//R89UvwMgog64NdjA6A\n\tzNxdk+ed4fwoBW1mW2SA5UeHGKN90FksfoGCcqeM=","Date":"Thu, 23 Sep 2021 18:59:02 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain ","Message-ID":"<20210923095901.GQ4382@pyrite.rasen.tech>","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-3-umang.jain@ideasonboard.com>\n\t<20210923074044.GP4382@pyrite.rasen.tech>\n\t<09fafc15-3475-8aa4-c3aa-c12a8c5aa986@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<09fafc15-3475-8aa4-c3aa-c12a8c5aa986@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19808,"web_url":"https://patchwork.libcamera.org/comment/19808/","msgid":"","date":"2021-09-23T10:14:50","subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Thu, Sep 23, 2021 at 12:54:53PM +0530, Umang Jain wrote:\n> Generation of thumbnail is not occuring currently because\n> ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed\n> to PostProcessorJpeg::process(). The commit 1264628d3c92(\"android:\n> jpeg: Configure thumbnailer based on request metadata\") introduced\n> the mechanism to retrieve the thumbanil size from request metadata,\n> however it didn't add the counterpart i.e. inserting the size in\n> the request metadata in request metadata template, at the first place.\n> \n> The patch fixes this issue by setting ANDROID_JPEG_THUMBNAIL_SIZE in\n> the request metadata template populated by\n> CameraCapabilities::requestTemplatePreview(). The value for\n> ANDROID_JPEG_THUMBNAIL_SIZE is set to be the first non-zero size\n> reported by static metadata ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES.\n> \n> Fixes: 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request metadata\")\n> Signed-off-by: Umang Jain \n> Reviewed-by: Laurent Pinchart \n> Reviewed-by: Hirokazu Honda \n> ---\n> src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++-\n> 1 file changed, 23 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 238b44db..ba6adf73 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -1347,7 +1347,7 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> \t * CameraMetadata is capable to resize the container on the fly, if the\n> \t * number of entries get exceeded.\n> \t */\n> -\tauto requestTemplate = std::make_unique(21, 36);\n> +\tauto requestTemplate = std::make_unique(22, 38);\n> \tif (!requestTemplate->isValid()) {\n> \t\treturn nullptr;\n> \t}\n> @@ -1368,6 +1368,28 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n> \t\t\t\t entry.data.i32, 2);\n> \n> +\t/*\n> +\t * Get thumbnail sizes from static metadata and add the first non-zero\n> +\t * size to the template.\n> +\t */\n> +\tfound = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n> +\t\t\t\t\t &entry);\n> +\tASSERT(found && entry.count >= 4);\n> +\tunsigned int j = 0;\n\nAnything wrong with i as a loop index ?\n\n> +\twhile (j < entry.count / 2) {\n> +\t\tif (entry.data.i32[j] == 0 || entry.data.i32[j + 1] == 0) {\n> +\t\t\tj += 2;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> +\t\t\t\t\t entry.data.i32 + j, 2);\n> +\t\tbreak;\n> +\t}\n\nI think that's overkill really. Android requires the first entry to be\n{ 0, 0 }. You can take the second entry unconditionally.\n\n> +\n> +\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> +\t\t\t\t entry.data.i32 + 2, 2);\n> +\n> \tuint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\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 1F363BF01C\n\tfor ;\n\tThu, 23 Sep 2021 10:14:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B2FA6918C;\n\tThu, 23 Sep 2021 12:14:53 +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 0197569189\n\tfor ;\n\tThu, 23 Sep 2021 12:14:52 +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 60B2545E;\n\tThu, 23 Sep 2021 12:14:52 +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=\"KRNiYys1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632392092;\n\tbh=5AABBrnC3ihd4fKZBuzsFbbd01yd6mmSFYp75+uneQs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KRNiYys1wewGZp1g2GiPmT1x37mMVnIQzNSpXl79QKzHDKMDnAftB6NtoDZ3etOvN\n\tV+c8C7YBcFcMY/4jY58G1uHjtCmIKRc1FwgPT3zuYfWL1yI6pGr+b8I7N0DYZ8U9/V\n\tuH0eiMgYEWxQxlfh9JpHSryu6aBYLOnqt2kqJEOc=","Date":"Thu, 23 Sep 2021 13:14:50 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210923072453.130346-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19810,"web_url":"https://patchwork.libcamera.org/comment/19810/","msgid":"","date":"2021-09-23T11:46:04","subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nOn 9/23/21 3:29 PM, paul.elder@ideasonboard.com wrote:\n> Hi Umang,\n>\n> On Thu, Sep 23, 2021 at 01:23:53PM +0530, Umang Jain wrote:\n>> Hi Paul,\n>>\n>> On 9/23/21 1:10 PM, paul.elder@ideasonboard.com wrote:\n>>> Hi Umang,\n>>>\n>>> On Thu, Sep 23, 2021 at 12:54:53PM +0530, Umang Jain wrote:\n>>>> Generation of thumbnail is not occuring currently because\n>>>> ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed\n>>>> to PostProcessorJpeg::process(). The commit 1264628d3c92(\"android:\n>>>> jpeg: Configure thumbnailer based on request metadata\") introduced\n>>>> the mechanism to retrieve the thumbanil size from request metadata,\n>>>> however it didn't add the counterpart i.e. inserting the size in\n>>>> the request metadata in request metadata template, at the first place.\n>>>>\n>>>> The patch fixes this issue by setting ANDROID_JPEG_THUMBNAIL_SIZE in\n>>>> the request metadata template populated by\n>>>> CameraCapabilities::requestTemplatePreview(). The value for\n>>>> ANDROID_JPEG_THUMBNAIL_SIZE is set to be the first non-zero size\n>>>> reported by static metadata ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES.\n>>>>\n>>>> Fixes: 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request metadata\")\n>>>> Signed-off-by: Umang Jain \n>>>> Reviewed-by: Laurent Pinchart \n>>>> Reviewed-by: Hirokazu Honda \n>>>> ---\n>>>> src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++-\n>>>> 1 file changed, 23 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n>>>> index 238b44db..ba6adf73 100644\n>>>> --- a/src/android/camera_capabilities.cpp\n>>>> +++ b/src/android/camera_capabilities.cpp\n>>>> @@ -1347,7 +1347,7 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n>>>> \t * CameraMetadata is capable to resize the container on the fly, if the\n>>>> \t * number of entries get exceeded.\n>>>> \t */\n>>>> -\tauto requestTemplate = std::make_unique(21, 36);\n>>>> +\tauto requestTemplate = std::make_unique(22, 38);\n>>>> \tif (!requestTemplate->isValid()) {\n>>>> \t\treturn nullptr;\n>>>> \t}\n>>>> @@ -1368,6 +1368,28 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n>>>> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n>>>> \t\t\t\t entry.data.i32, 2);\n>>>> +\t/*\n>>>> +\t * Get thumbnail sizes from static metadata and add the first non-zero\n>>>> +\t * size to the template.\n>>>> +\t */\n>>>> +\tfound = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n>>>> +\t\t\t\t\t &entry);\n>>>> +\tASSERT(found && entry.count >= 4);\n>>>> +\tunsigned int j = 0;\n>>>> +\twhile (j < entry.count / 2) {\n>>> Since you're incrementing j by 2, shouldn't the limit be entry.count?\n>>\n>> My understanding is 2 entries make 1 size, no?\n>> ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES be like for e.g.\n>>\n>>     {0, 0, w1, h1, w2, h2}\n>>\n>> So entry.count is 6 but sizes are 3\n>>\n>> To go to next Size, incremement by 2 so, j += 2\n> That's true, but for example:\n>\n> {0, 0, w1, h1, w2, h2}\n>\n> Iteration 1: j = 0, so you take [0, 1]; afterward j += 2\n> Iteration 2: j = 2, so you take [2, 3]; afterward j += 2\n> Iteration 3: j = 4, so you take [4, 5]; afterward j += 2\n> Itreation 4: j = 6, fails j < entry.count; skip\n>\n> If you stop at entry.count / 2 then you'll skip the second half of\n> pairs.\n\n\nOh, you are correct. I missed to see that somehow :-S\n\n>\n>\n> Paul\n>\n>> Does it make sense?\n>>\n>>\n>>>> +\t\tif (entry.data.i32[j] == 0 || entry.data.i32[j + 1] == 0) {\n>>>> +\t\t\tj += 2;\n>>>> +\t\t\tcontinue;\n>>>> +\t\t}\n>>>> +\n>>>> +\t\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>>>> +\t\t\t\t\t entry.data.i32 + j, 2);\n>>>> +\t\tbreak;\n>>>> +\t}\n>>>> +\n>>>> +\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>>>> +\t\t\t\t entry.data.i32 + 2, 2);\n>>> iirc adding an entry that already exists causes weird problems...\n>>\n>> Ah crap, this should be outside the loop. I am re-assigning it here. Sorry,\n>> rebase fiasco it seems.\n>>\n>>>\n>>> Paul\n>>>\n>>>> +\n>>>> \tuint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n>>>> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n>>>> -- \n>>>> 2.31.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 049F9BF01C\n\tfor ;\n\tThu, 23 Sep 2021 11:46:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4C1A06918E;\n\tThu, 23 Sep 2021 13:46:11 +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 AAFED69189\n\tfor ;\n\tThu, 23 Sep 2021 13:46:09 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.101])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A9E2945E;\n\tThu, 23 Sep 2021 13:46:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cEdNd4IG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632397569;\n\tbh=QHa8qnbZR7k+/u9fVUjZVGXHHKfxgjZ0nYP1z/FxLIk=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=cEdNd4IGLCTraPE0yeQ6ERePmOcH1hhGFqVg7wX7rlmyK2x4O3tGoAhrl82UNZdRD\n\tifgj7DSqJh5HqLJPdtmS6h3eNzyqC9V0t6mnzt2wD0I39uWzus+PvyeslLNoH05GZp\n\t0foAajoMBi7oUSCVtIJXTra3o5mycgyG5q2To88U=","To":"paul.elder@ideasonboard.com","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-3-umang.jain@ideasonboard.com>\n\t<20210923074044.GP4382@pyrite.rasen.tech>\n\t<09fafc15-3475-8aa4-c3aa-c12a8c5aa986@ideasonboard.com>\n\t<20210923095901.GQ4382@pyrite.rasen.tech>","From":"Umang Jain ","Message-ID":"","Date":"Thu, 23 Sep 2021 17:16:04 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210923095901.GQ4382@pyrite.rasen.tech>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19811,"web_url":"https://patchwork.libcamera.org/comment/19811/","msgid":"<4f3ccf54-07af-c113-31cc-4cea5d7ee443@ideasonboard.com>","date":"2021-09-23T11:48:08","subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 9/23/21 3:44 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Thu, Sep 23, 2021 at 12:54:53PM +0530, Umang Jain wrote:\n>> Generation of thumbnail is not occuring currently because\n>> ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed\n>> to PostProcessorJpeg::process(). The commit 1264628d3c92(\"android:\n>> jpeg: Configure thumbnailer based on request metadata\") introduced\n>> the mechanism to retrieve the thumbanil size from request metadata,\n>> however it didn't add the counterpart i.e. inserting the size in\n>> the request metadata in request metadata template, at the first place.\n>>\n>> The patch fixes this issue by setting ANDROID_JPEG_THUMBNAIL_SIZE in\n>> the request metadata template populated by\n>> CameraCapabilities::requestTemplatePreview(). The value for\n>> ANDROID_JPEG_THUMBNAIL_SIZE is set to be the first non-zero size\n>> reported by static metadata ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES.\n>>\n>> Fixes: 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request metadata\")\n>> Signed-off-by: Umang Jain \n>> Reviewed-by: Laurent Pinchart \n>> Reviewed-by: Hirokazu Honda \n>> ---\n>> src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++-\n>> 1 file changed, 23 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n>> index 238b44db..ba6adf73 100644\n>> --- a/src/android/camera_capabilities.cpp\n>> +++ b/src/android/camera_capabilities.cpp\n>> @@ -1347,7 +1347,7 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n>> \t * CameraMetadata is capable to resize the container on the fly, if the\n>> \t * number of entries get exceeded.\n>> \t */\n>> -\tauto requestTemplate = std::make_unique(21, 36);\n>> +\tauto requestTemplate = std::make_unique(22, 38);\n>> \tif (!requestTemplate->isValid()) {\n>> \t\treturn nullptr;\n>> \t}\n>> @@ -1368,6 +1368,28 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n>> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n>> \t\t\t\t entry.data.i32, 2);\n>> \n>> +\t/*\n>> +\t * Get thumbnail sizes from static metadata and add the first non-zero\n>> +\t * size to the template.\n>> +\t */\n>> +\tfound = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n>> +\t\t\t\t\t &entry);\n>> +\tASSERT(found && entry.count >= 4);\n>> +\tunsigned int j = 0;\n> Anything wrong with i as a loop index ?\n>\n>> +\twhile (j < entry.count / 2) {\n>> +\t\tif (entry.data.i32[j] == 0 || entry.data.i32[j + 1] == 0) {\n>> +\t\t\tj += 2;\n>> +\t\t\tcontinue;\n>> +\t\t}\n>> +\n>> +\t\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>> +\t\t\t\t\t entry.data.i32 + j, 2);\n>> +\t\tbreak;\n>> +\t}\n> I think that's overkill really. Android requires the first entry to be\n> { 0, 0 }. You can take the second entry unconditionally.\n\n\nOk, I am keeping the ASSERT and dropping the loop, similar to v2.\n\nI'll merge the patches after validating CTS (which could take a while to \nget in)\n\n>\n>> +\n>> +\trequestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>> +\t\t\t\t entry.data.i32 + 2, 2);\n>> +\n>> \tuint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n>> \trequestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\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 480B9BF01C\n\tfor ;\n\tThu, 23 Sep 2021 11:48:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84BA96918C;\n\tThu, 23 Sep 2021 13:48:14 +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 2192A69189\n\tfor ;\n\tThu, 23 Sep 2021 13:48:13 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.101])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E7F7945E;\n\tThu, 23 Sep 2021 13:48:11 +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=\"JNfCXo0B\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632397692;\n\tbh=tEX2nmJ/NZ5WnJJUvtE7uXYqXpaG9oYTQn8qJPbuE/U=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=JNfCXo0BElwtd1w9fXogtUB94hf4JnJOTSL2Z2nmf/TEQbRtTsVd9inEq8ptEcWTl\n\tBH1/z/h8nkXrix+S9faxrbtROZxrOrav8swlulJxtgxOI16L1SoDgsI9Wyv8FnZv5h\n\tdqTL5LuHegioc/quWbNYIfsp5QcAsoEFJiXN+NLI=","To":"Laurent Pinchart ","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-3-umang.jain@ideasonboard.com>\n\t","From":"Umang Jain ","Message-ID":"<4f3ccf54-07af-c113-31cc-4cea5d7ee443@ideasonboard.com>","Date":"Thu, 23 Sep 2021 17:18:08 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] android: Fix generation of\n\tthumbnail for EXIF data","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]