[{"id":19603,"web_url":"https://patchwork.libcamera.org/comment/19603/","msgid":"","date":"2021-09-10T12:54:01","subject":"Re: [libcamera-devel] [PATCH v1 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Umang, thank you for the patch.\n\nOn Fri, Sep 10, 2021 at 7:47 PM Umang Jain wrote:\n>\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(). This is a regression introduced in\n> 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request\n> metadata\").\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> ---\n> src/android/camera_capabilities.cpp | 17 +++++++++++++++--\n> 1 file changed, 15 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index e92bca42..ba551b86 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -1341,9 +1341,9 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> {\n> /*\n> * \\todo Keep this in sync with the actual number of entries.\n> - * Currently: 20 entries, 35 bytes\n> + * Currently: 21 entries, 37 bytes\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> @@ -1364,6 +1364,19 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n> entry.data.i32, 2);\n>\n> + /*\n> + * Get thumbnail sizes from static metadata and add the first non-zero\n> + * size to the template.\n> + */\n> + found = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n> + &entry);\n> + if (!found) {\n> + LOG(HAL, Error) << \"availableThumbnailSizes not found in static metadata\";\n> + return nullptr;\n> + }\n\nIs it possible to check if there is the second element?\n\n-Hiro\n> + requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> + entry.data.i32 + 2, 2);\n> +\n> uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n> requestTemplate->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 426C0BDC71\n\tfor ;\n\tFri, 10 Sep 2021 12:54:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB0106917E;\n\tFri, 10 Sep 2021 14:54:13 +0200 (CEST)","from mail-ej1-x634.google.com (mail-ej1-x634.google.com\n\t[IPv6:2a00:1450:4864:20::634])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3BDD169169\n\tfor ;\n\tFri, 10 Sep 2021 14:54:12 +0200 (CEST)","by mail-ej1-x634.google.com with SMTP id bt14so4091643ejb.3\n\tfor ;\n\tFri, 10 Sep 2021 05:54:12 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"W9oPVB/E\"; 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=socqW9mcyrsyOnHCCx6jYzpSH7/EOM1CQlRdJJm/sYY=;\n\tb=W9oPVB/EyOqlItZOWcMi8m2FdM2P5Sl5ryScliHW5GvCiQewIsif/ciTUFXXyCdp91\n\t1sTDPt90unEi/6S7NiFo3xCLBPjMOvJ0YZvbnEkEN45iAolni+BWOR0QBoiA2VZ+yqNs\n\tASowLLmuU3E9KO4kxTU2F75iqwmEZB06whuGs=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=socqW9mcyrsyOnHCCx6jYzpSH7/EOM1CQlRdJJm/sYY=;\n\tb=kwjxHpxe6/DTKpX107lEK7c5MFGRGW4qvjpDGzQh6/M7a/ioiD+MU49bm3m/AcHNgL\n\tQknvz8/MxRpatze9YhhFq6AbVn/vu0PN4pNxn2qweo5whoNZ5AKIPKTbWye6J2Vu7o8f\n\tYuy9M4EDktXhinj+CqSQtihlkczAwXn8hTC2kMeA+IkBK8Bge2A4/+qrKtIw1mcbMt0L\n\ta9LXzaTHzwxrwP2j5+lBtAh+bc1fFWRhh4Y9WqmSFsvMYszhOvoc4i7JDDT0+4UoZ1Ps\n\tsVKN70boAcBiZ88Dw8EptPorNoaXQCAiVGBrh/nFgAy1w1cKbbvj1X3kNtyeSMPzsogU\n\tHNLw==","X-Gm-Message-State":"AOAM530uS7p/cta+VKQc+L4IUyxcv0HYPC3zra0qz7YXp8hJhe2083G9\n\trXFdcbv3oEdQqX4j3gt2tp7poKTpXSdxK/iW3GvcvqG0XFI=","X-Google-Smtp-Source":"ABdhPJyRez8/yviXUYjp0lkO2t0z4R4+30V0KFYkSqK41L71EJD1vHyx1UW0JhkRDqU0TmKITWmT9285BeoZbb9drjE=","X-Received":"by 2002:a17:906:f289:: with SMTP id\n\tgu9mr9056935ejb.559.1631278451885; \n\tFri, 10 Sep 2021 05:54:11 -0700 (PDT)","MIME-Version":"1.0","References":"<20210910104729.542779-1-umang.jain@ideasonboard.com>\n\t<20210910104729.542779-3-umang.jain@ideasonboard.com>","In-Reply-To":"<20210910104729.542779-3-umang.jain@ideasonboard.com>","From":"Hirokazu Honda ","Date":"Fri, 10 Sep 2021 21:54:01 +0900","Message-ID":"","To":"Umang Jain ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19605,"web_url":"https://patchwork.libcamera.org/comment/19605/","msgid":"<83474312-1245-bb45-0df6-5c9bea892120@ideasonboard.com>","date":"2021-09-10T14:31:50","subject":"Re: [libcamera-devel] [PATCH v1 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 Hiro\n\nOn 9/10/21 6:24 PM, Hirokazu Honda wrote:\n> Hi Umang, thank you for the patch.\n>\n> On Fri, Sep 10, 2021 at 7:47 PM 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(). This is a regression introduced in\n>> 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request\n>> metadata\").\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>> ---\n>> src/android/camera_capabilities.cpp | 17 +++++++++++++++--\n>> 1 file changed, 15 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n>> index e92bca42..ba551b86 100644\n>> --- a/src/android/camera_capabilities.cpp\n>> +++ b/src/android/camera_capabilities.cpp\n>> @@ -1341,9 +1341,9 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n>> {\n>> /*\n>> * \\todo Keep this in sync with the actual number of entries.\n>> - * Currently: 20 entries, 35 bytes\n>> + * Currently: 21 entries, 37 bytes\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>> @@ -1364,6 +1364,19 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n>> requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n>> entry.data.i32, 2);\n>>\n>> + /*\n>> + * Get thumbnail sizes from static metadata and add the first non-zero\n>> + * size to the template.\n>> + */\n>> + found = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n>> + &entry);\n>> + if (!found) {\n>> + LOG(HAL, Error) << \"availableThumbnailSizes not found in static metadata\";\n>> + return nullptr;\n>> + }\n> Is it possible to check if there is the second element?\n\nSecond element ? Do you mean second (or other sizes) being reported in \nANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES?\n\nIt should be plausible to extract that from the entry's data I think.  \nIs something unclear?\n\n> -Hiro\n>> + requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n>> + entry.data.i32 + 2, 2);\n>> +\n>> uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n>> requestTemplate->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 8607ABDC71\n\tfor ;\n\tFri, 10 Sep 2021 14:31:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 88A9769170;\n\tFri, 10 Sep 2021 16:31:57 +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 5A29669169\n\tfor ;\n\tFri, 10 Sep 2021 16:31:55 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.149])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5482D9E;\n\tFri, 10 Sep 2021 16:31:54 +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=\"jEiuSbUG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631284314;\n\tbh=1adl2YHWvshC99KAz+1Pp/f5b8iD0HhX8is5ncR+tCQ=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=jEiuSbUGhKcNegCK37z7IRJ2C4ww9ItCB0SpSIvA4jRwxQe+o/wgR4YjF4oc7BRYL\n\thDaKmZiitSYoOCl/z2mlJwGPW1h5JZStfgekw7dbmpcCMDH27m7p6xT4dxooHgiIzP\n\tyk1gRdasZsI/T6bqViX+zdstEX5oXXls/Lnt7u0s=","To":"Hirokazu Honda ","References":"<20210910104729.542779-1-umang.jain@ideasonboard.com>\n\t<20210910104729.542779-3-umang.jain@ideasonboard.com>\n\t","From":"Umang Jain ","Message-ID":"<83474312-1245-bb45-0df6-5c9bea892120@ideasonboard.com>","Date":"Fri, 10 Sep 2021 20:01:50 +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":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v1 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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19613,"web_url":"https://patchwork.libcamera.org/comment/19613/","msgid":"","date":"2021-09-10T16:14:12","subject":"Re: [libcamera-devel] [PATCH v1 2/2] android: Fix generation of\n\tthumbnail for EXIF data","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Umang,\n\nOn Fri, Sep 10, 2021 at 11:31 PM Umang Jain wrote:\n>\n> Hi Hiro\n>\n> On 9/10/21 6:24 PM, Hirokazu Honda wrote:\n> > Hi Umang, thank you for the patch.\n> >\n> > On Fri, Sep 10, 2021 at 7:47 PM 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(). This is a regression introduced in\n> >> 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request\n> >> metadata\").\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> >> ---\n> >> src/android/camera_capabilities.cpp | 17 +++++++++++++++--\n> >> 1 file changed, 15 insertions(+), 2 deletions(-)\n> >>\n> >> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> >> index e92bca42..ba551b86 100644\n> >> --- a/src/android/camera_capabilities.cpp\n> >> +++ b/src/android/camera_capabilities.cpp\n> >> @@ -1341,9 +1341,9 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> >> {\n> >> /*\n> >> * \\todo Keep this in sync with the actual number of entries.\n> >> - * Currently: 20 entries, 35 bytes\n> >> + * Currently: 21 entries, 37 bytes\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> >> @@ -1364,6 +1364,19 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> >> requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n> >> entry.data.i32, 2);\n> >>\n> >> + /*\n> >> + * Get thumbnail sizes from static metadata and add the first non-zero\n> >> + * size to the template.\n> >> + */\n> >> + found = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n> >> + &entry);\n> >> + if (!found) {\n> >> + LOG(HAL, Error) << \"availableThumbnailSizes not found in static metadata\";\n> >> + return nullptr;\n> >> + }\n> > Is it possible to check if there is the second element?\n>\n> Second element ? Do you mean second (or other sizes) being reported in\n> ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES?\n>\n> It should be plausible to extract that from the entry's data I think.\n> Is something unclear?\n>\n\nI meant if SIZES returns empty size (i.e. the sizes are one size\nlist), accessing entry.data.i32+2 is invalid.\nI wonder if we should check the size of the returned size list.\n\n-Hiro\n> > -Hiro\n> >> + requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> >> + entry.data.i32 + 2, 2);\n> >> +\n> >> uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n> >> requestTemplate->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 2BA48BDB1D\n\tfor ;\n\tFri, 10 Sep 2021 16:14:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 974906917E;\n\tFri, 10 Sep 2021 18:14:24 +0200 (CEST)","from mail-ed1-x533.google.com (mail-ed1-x533.google.com\n\t[IPv6:2a00:1450:4864:20::533])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7FE3D69169\n\tfor ;\n\tFri, 10 Sep 2021 18:14:23 +0200 (CEST)","by mail-ed1-x533.google.com with SMTP id g21so3217514edw.4\n\tfor ;\n\tFri, 10 Sep 2021 09:14:23 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"PcHYAJPO\"; 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=h/udfIf++xBkzVbMI0ib2MqoVGITS5XUGugUpN3CifY=;\n\tb=PcHYAJPO8F0OTBSXk/4NwOdBD1Ei2NI1Q8EIVRi3b+dq5COnG4acuef4P2WQ9W5NNn\n\t0FoC6vJVri5V2/wxMXA1/TD/AN3PDC4trqOhdDokt0cn1dE0xDlVkuoHyI7n40KLt7e5\n\tFBm5K9BaRZ7nOC0jTQ7hkchaJLdGdOIZqodgc=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=h/udfIf++xBkzVbMI0ib2MqoVGITS5XUGugUpN3CifY=;\n\tb=FUTrejgFfDj2jaTrQaKTDCq6d02FURaud/N2gWiVMKNQbZcvc6nxkaPYOhSZA40Hzm\n\tvStWvsOAJgUWb/msEYiq/oPmE4XwDPAisxVm3moiFkGWKhttnMDYPJBnBYkunL+eR/R/\n\tOn2vfT0nYm54KPXh4LR+sM+1EHnuiAwOBBAju7qavftzLlYhCou6BJGF2hVhTz2aAZXs\n\tsC8sU0ZW/L/M99mKw9/ADOICHH8FwpDgA6BH4MtLcFZNl0uwTUw/EatkqnbiMpd2i2JD\n\tOO1wZlmWVnqQFez4Aqbd5Iht3IOcUzZW7exJJFO2kX58pbcbfBDFoT96aP9he7opNiD6\n\tpcjg==","X-Gm-Message-State":"AOAM532fnvQ0i61Rm/rHizh58qgYZsSTuHzDafbxLD0wfaykDPuyQMYQ\n\tyzmswf3MlJRcr8O1hIe2hKsb9HwL60qDScUwItcIsyQSLW0=","X-Google-Smtp-Source":"ABdhPJyWIHoRVe8w1RZkSgxqt78jEudHcZgnq1Jpbff7M8jYc6WFG40QqX4PE3cK8J8AkLTjoVXg6KgEUgIsFau5sh0=","X-Received":"by 2002:aa7:c9c1:: with SMTP id i1mr9939730edt.204.1631290463153;\n\tFri, 10 Sep 2021 09:14:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20210910104729.542779-1-umang.jain@ideasonboard.com>\n\t<20210910104729.542779-3-umang.jain@ideasonboard.com>\n\t\n\t<83474312-1245-bb45-0df6-5c9bea892120@ideasonboard.com>","In-Reply-To":"<83474312-1245-bb45-0df6-5c9bea892120@ideasonboard.com>","From":"Hirokazu Honda ","Date":"Sat, 11 Sep 2021 01:14:12 +0900","Message-ID":"","To":"Umang Jain ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19618,"web_url":"https://patchwork.libcamera.org/comment/19618/","msgid":"","date":"2021-09-10T16:31:31","subject":"Re: [libcamera-devel] [PATCH v1 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":"Hello,\n\nOn Sat, Sep 11, 2021 at 01:14:12AM +0900, Hirokazu Honda wrote:\n> On Fri, Sep 10, 2021 at 11:31 PM Umang Jain wrote:\n> > On 9/10/21 6:24 PM, Hirokazu Honda wrote:\n> > > On Fri, Sep 10, 2021 at 7:47 PM 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(). This is a regression introduced in\n> > >> 1264628d3c92(\"android: jpeg: Configure thumbnailer based on request\n\ns/2(/2 (/\n\n> > >> metadata\").\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> > >> ---\n> > >> src/android/camera_capabilities.cpp | 17 +++++++++++++++--\n> > >> 1 file changed, 15 insertions(+), 2 deletions(-)\n> > >>\n> > >> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > >> index e92bca42..ba551b86 100644\n> > >> --- a/src/android/camera_capabilities.cpp\n> > >> +++ b/src/android/camera_capabilities.cpp\n> > >> @@ -1341,9 +1341,9 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> > >> {\n> > >> /*\n> > >> * \\todo Keep this in sync with the actual number of entries.\n> > >> - * Currently: 20 entries, 35 bytes\n> > >> + * Currently: 21 entries, 37 bytes\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> > >> @@ -1364,6 +1364,19 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> > >> requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n> > >> entry.data.i32, 2);\n> > >>\n> > >> + /*\n> > >> + * Get thumbnail sizes from static metadata and add the first non-zero\n> > >> + * size to the template.\n> > >> + */\n> > >> + found = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,\n> > >> + &entry);\n> > >> + if (!found) {\n> > >> + LOG(HAL, Error) << \"availableThumbnailSizes not found in static metadata\";\n> > >> + return nullptr;\n> > >> + }\n> > >\n> > > Is it possible to check if there is the second element?\n> >\n> > Second element ? Do you mean second (or other sizes) being reported in\n> > ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES?\n> >\n> > It should be plausible to extract that from the entry's data I think.\n> > Is something unclear?\n> \n> I meant if SIZES returns empty size (i.e. the sizes are one size\n> list), accessing entry.data.i32+2 is invalid.\n> I wonder if we should check the size of the returned size list.\n\nIt's not supposed to happen, but if we want to be defensive, we can add\n\n\tASSERT(entry.count >= 4);\n\nActually, as ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES is supposed to\nalways be there, I'd drop the error message and write\n\n\tASSERT(found && entry.count >= 4);\n\nReviewed-by: Laurent Pinchart \n\n> > >> + requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,\n> > >> + entry.data.i32 + 2, 2);\n> > >> +\n> > >> uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;\n> > >> requestTemplate->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 EADCABDC71\n\tfor ;\n\tFri, 10 Sep 2021 16:31:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B08D6917C;\n\tFri, 10 Sep 2021 18:31:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3141969169\n\tfor ;\n\tFri, 10 Sep 2021 18:31:54 +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 988C5883;\n\tFri, 10 Sep 2021 18:31:53 +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=\"Pe917QNl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631291513;\n\tbh=jcUsiVqaqNylLXoT+mjCrSFEIJz8JEV/XsJHSyWfnNk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Pe917QNlFzPuFst3jJ87Y+3jkjaLNvSsfLp14D51/hWlIKE+DnNHBZrpqmMU2YTl/\n\t3nDMpkTY3TVm3gjKLblMDGFWHSoHTp9QC+5TLC0AN+qs5nkXqx2GJCgM8ysniEmbXQ\n\t/TLH3cHfN9iaQ0uDeqjfRRNVszTBui3RjxMgOfcU=","Date":"Fri, 10 Sep 2021 19:31:31 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210910104729.542779-1-umang.jain@ideasonboard.com>\n\t<20210910104729.542779-3-umang.jain@ideasonboard.com>\n\t\n\t<83474312-1245-bb45-0df6-5c9bea892120@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1 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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]