<div dir="ltr"><div dir="ltr">Hi Paul, </div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 2, 2021 at 6:52 PM <<a href="mailto:paul.elder@ideasonboard.com">paul.elder@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Hiro,<br>
<br>
On Wed, Jun 02, 2021 at 04:57:58PM +0900, Hirokazu Honda wrote:<br>
> Hi Paul,<br>
> <br>
> On Wed, Jun 2, 2021 at 3:32 PM <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>> wrote:<br>
> <br>
> Hi Hiro,<br>
> <br>
> On Wed, Jun 02, 2021 at 02:01:24PM +0900, Hirokazu Honda wrote:<br>
> > Hi Paul, thank you for the patch.<br>
> ><br>
> > On Wed, Jun 2, 2021 at 12:32 PM <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>> wrote:<br>
> ><br>
> > On Wed, Jun 02, 2021 at 12:30:21PM +0900, <a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a><br>
> > wrote:<br>
> > > Hi Umang,<br>
> > ><br>
> > > On Tue, Jun 01, 2021 at 04:54:55PM +0530, Umang Jain wrote:<br>
> > > > Report ANDROID_SENSOR_FRAME_DURATION as an available key for CTS<br>
> to<br>
> > ><br>
> > > s/available key/available result key/<br>
> > ><br>
> > > It's also a valid request key, and this patch doesn't add that, so<br>
> I<br>
> > > think it should be specified (also in the subject).<br>
> ><br>
> > By "specified" I mean that "result" should be specified.<br>
> ><br>
> > As for the subject, s/key/result key/<br>
> ><br>
> ><br>
> > Paul<br>
> ><br>
> > ><br>
> > > > read out the value of frame duration we set in<br>
> > CameraDevice::getResultMetadata().<br>
> > > > Failing to do so might fail the CTS test:<br>
> > > > - android.hardware.camera2.cts.CaptureRequestTest#<br>
> > testNoiseReductionModeControl<br>
> > > ><br>
> > > > Fixes: 3beb1accac1d ("android: camera_device: Fix sensor frame<br>
> > duration")<br>
> > > > Signed-off-by: Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank">umang.jain@ideasonboard.com</a>><br>
> ><br>
> ><br>
> > First, this code adds the missing request result key. So<br>
> > Reviewed-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org" target="_blank">hiroh@chromium.org</a>><br>
> ><br>
> > However, although this is not necessarily related to this change, I<br>
> wonder if<br>
> > those available keys should be there if and only if an entry with the key<br>
> is<br>
> > actually added.<br>
> <br>
> I think you're right. We might need to rethink where the keys and values<br>
> come from :/<br>
> <br>
> I guess if we declare that we support some key then the application<br>
> can expect that it's always present.<br>
> <br>
> <br>
> <br>
> I wonder if we should dynamically construct the available keys upon<br>
> CameraMetadata::get() from keys that have been added.<br>
<br>
The static metadata is constructed and returned way before any result<br>
metadata is ever returned though.<br>
<br>
I'm currently redesigning how we populate the static metadata based on<br>
Camera capabilities (eg. what libcamera controls the camera supports).<br>
It'll probably also connect into what we return in the result metadata,<br>
and what we accept in the request metadata. These both will of course<br>
affect the available keys that we report in the static metadata.<br>
I'll keep this in mind in the redesign.<br>
<br></blockquote><div><br></div><div>Ack. I look forward to your redesign.</div><div><br></div><div>Thanks,</div><div>-Hiro</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Paul<br>
<br>
> We should look up the hard-code tables, if there is no smart way, to find what<br>
> each key is added to, ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS,<br>
> ANDROID_REQUEST_AVAILABLE_RESULT_KEYS or<br>
> ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS.<br>
> <br>
> -Hiro<br>
> <br>
> > Could you run android.hardware.camera2.cts.CaptureResultTest while some<br>
> entry<br>
> > is dropped and the entry is in available keys?<br>
> <br>
> android.hardware.camera2.cts.CaptureRequestTest#<br>
> testNoiseReductionModeControl<br>
> fails complaining that the key that it expects to be present is not. The<br>
> other tests don't seem to care.<br>
> <br>
> <br>
> Paul<br>
> <br>
> ><br>
> > > > ---<br>
> > > > src/android/camera_device.cpp | 1 +<br>
> > > > 1 file changed, 1 insertion(+)<br>
> > > ><br>
> > > > diff --git a/src/android/camera_device.cpp b/src/android/<br>
> > camera_device.cpp<br>
> > > > index fddc07ff..fe332ec3 100644<br>
> > > > --- a/src/android/camera_device.cpp<br>
> > > > +++ b/src/android/camera_device.cpp<br>
> > > > @@ -1422,6 +1422,7 @@ const camera_metadata_t<br>
> > *CameraDevice::getStaticMetadata()<br>
> > > > ANDROID_REQUEST_PIPELINE_DEPTH,<br>
> > > > ANDROID_SCALER_CROP_REGION,<br>
> > > > ANDROID_SENSOR_EXPOSURE_TIME,<br>
> > > > + ANDROID_SENSOR_FRAME_DURATION,<br>
> > > > ANDROID_SENSOR_ROLLING_SHUTTER_SKEW,<br>
> > > > ANDROID_SENSOR_TEST_PATTERN_MODE,<br>
> > > > ANDROID_SENSOR_TIMESTAMP,<br>
> > > > --<br>
> > > > 2.31.1<br>
> > > ><br>
> ><br>
> <br>
</blockquote></div></div>