| Message ID | 20211203071512.1197248-1-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 37c41aa6a69985f99f9540dc89d094df61770fdc |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, On Fri, Dec 03, 2021 at 12:45:12PM +0530, Umang Jain wrote: > We have some stream resolution which can provide slightly better > frame duration than what we cap (i.e. 1/30 fps). The problem with > this is CTS complains if the camera goes faster during the test > than minFrameDuration reported for that resolution. For instance, > > 1080p minFrameDuration: > - Nautilus : 33282000ns > - Soraka : 33147000ns > > Both are less than capped minFrameDuration 1/30 fps (33333333.33ns). > > This patch considers this situation and doesn't cap the > minFrameDuration if the hardware can provide frame durations slightly > better. The tolerance considered is 1% only from the cap. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > On LIMITED level - no regressions were found : 230/231 pass rate Has this been run on Soraka too ? > > On FULL level - this fixes the test: > android.hardware.camera2.cts.SurfaceViewPreviewTest#testPreviewFpsRange Great! Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > src/android/camera_capabilities.cpp | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > index f357902e..c4c26089 100644 > --- a/src/android/camera_capabilities.cpp > +++ b/src/android/camera_capabilities.cpp > @@ -648,7 +648,7 @@ int CameraCapabilities::initializeStreamConfigurations() > int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; > > /* > - * Cap min frame duration to 30 FPS. > + * Cap min frame duration to 30 FPS with 1% tolerance. > * > * 30 frames per second has been validated as the most > * opportune frame rate for quality tuning, and power > @@ -667,8 +667,18 @@ int CameraCapabilities::initializeStreamConfigurations() > * control to be specified for each Request. Defer this > * to the in-development configuration API rework. > */ > - if (minFrameDuration < 1e9 / 30.0) > - minFrameDuration = 1e9 / 30.0; > + int64_t minFrameDurationCap = 1e9 / 30.0; > + if (minFrameDuration < minFrameDurationCap) { > + float tolerance = > + (minFrameDurationCap - minFrameDuration) * 100.0 / minFrameDurationCap; > + > + /* > + * If the tolerance is less than 1%, do not cap > + * the frame duration. > + */ > + if (tolerance > 1.0) > + minFrameDuration = minFrameDurationCap; > + } > > streamConfigurations_.push_back({ > res, androidFormat, minFrameDuration, maxFrameDuration, > -- > 2.31.0 >
Hi Jacopo, On 12/3/21 1:44 PM, Jacopo Mondi wrote: > Hi Umang, > > On Fri, Dec 03, 2021 at 12:45:12PM +0530, Umang Jain wrote: >> We have some stream resolution which can provide slightly better >> frame duration than what we cap (i.e. 1/30 fps). The problem with >> this is CTS complains if the camera goes faster during the test >> than minFrameDuration reported for that resolution. For instance, >> >> 1080p minFrameDuration: >> - Nautilus : 33282000ns >> - Soraka : 33147000ns >> >> Both are less than capped minFrameDuration 1/30 fps (33333333.33ns). >> >> This patch considers this situation and doesn't cap the >> minFrameDuration if the hardware can provide frame durations slightly >> better. The tolerance considered is 1% only from the cap. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> On LIMITED level - no regressions were found : 230/231 pass rate > Has this been run on Soraka too ? Not yet, to be done as part of integration/ merge process. > >> On FULL level - this fixes the test: >> android.hardware.camera2.cts.SurfaceViewPreviewTest#testPreviewFpsRange > Great! > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks > j > > >> --- >> src/android/camera_capabilities.cpp | 16 +++++++++++++--- >> 1 file changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp >> index f357902e..c4c26089 100644 >> --- a/src/android/camera_capabilities.cpp >> +++ b/src/android/camera_capabilities.cpp >> @@ -648,7 +648,7 @@ int CameraCapabilities::initializeStreamConfigurations() >> int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; >> >> /* >> - * Cap min frame duration to 30 FPS. >> + * Cap min frame duration to 30 FPS with 1% tolerance. >> * >> * 30 frames per second has been validated as the most >> * opportune frame rate for quality tuning, and power >> @@ -667,8 +667,18 @@ int CameraCapabilities::initializeStreamConfigurations() >> * control to be specified for each Request. Defer this >> * to the in-development configuration API rework. >> */ >> - if (minFrameDuration < 1e9 / 30.0) >> - minFrameDuration = 1e9 / 30.0; >> + int64_t minFrameDurationCap = 1e9 / 30.0; >> + if (minFrameDuration < minFrameDurationCap) { >> + float tolerance = >> + (minFrameDurationCap - minFrameDuration) * 100.0 / minFrameDurationCap; >> + >> + /* >> + * If the tolerance is less than 1%, do not cap >> + * the frame duration. >> + */ >> + if (tolerance > 1.0) >> + minFrameDuration = minFrameDurationCap; >> + } >> >> streamConfigurations_.push_back({ >> res, androidFormat, minFrameDuration, maxFrameDuration, >> -- >> 2.31.0 >>
Hi Umang, On Fri, Dec 03, 2021 at 12:45:12PM +0530, Umang Jain wrote: > We have some stream resolution which can provide slightly better > frame duration than what we cap (i.e. 1/30 fps). The problem with > this is CTS complains if the camera goes faster during the test > than minFrameDuration reported for that resolution. For instance, > > 1080p minFrameDuration: > - Nautilus : 33282000ns > - Soraka : 33147000ns > > Both are less than capped minFrameDuration 1/30 fps (33333333.33ns). > > This patch considers this situation and doesn't cap the > minFrameDuration if the hardware can provide frame durations slightly > better. The tolerance considered is 1% only from the cap. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Looks good. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > On LIMITED level - no regressions were found : 230/231 pass rate > > On FULL level - this fixes the test: > android.hardware.camera2.cts.SurfaceViewPreviewTest#testPreviewFpsRange > --- > src/android/camera_capabilities.cpp | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > index f357902e..c4c26089 100644 > --- a/src/android/camera_capabilities.cpp > +++ b/src/android/camera_capabilities.cpp > @@ -648,7 +648,7 @@ int CameraCapabilities::initializeStreamConfigurations() > int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; > > /* > - * Cap min frame duration to 30 FPS. > + * Cap min frame duration to 30 FPS with 1% tolerance. > * > * 30 frames per second has been validated as the most > * opportune frame rate for quality tuning, and power > @@ -667,8 +667,18 @@ int CameraCapabilities::initializeStreamConfigurations() > * control to be specified for each Request. Defer this > * to the in-development configuration API rework. > */ > - if (minFrameDuration < 1e9 / 30.0) > - minFrameDuration = 1e9 / 30.0; > + int64_t minFrameDurationCap = 1e9 / 30.0; > + if (minFrameDuration < minFrameDurationCap) { > + float tolerance = > + (minFrameDurationCap - minFrameDuration) * 100.0 / minFrameDurationCap; > + > + /* > + * If the tolerance is less than 1%, do not cap > + * the frame duration. > + */ > + if (tolerance > 1.0) > + minFrameDuration = minFrameDurationCap; > + } > > streamConfigurations_.push_back({ > res, androidFormat, minFrameDuration, maxFrameDuration, > -- > 2.31.0 >
Hi Umang, Thank you for the patch. On Fri, Dec 03, 2021 at 12:45:12PM +0530, Umang Jain wrote: > We have some stream resolution which can provide slightly better > frame duration than what we cap (i.e. 1/30 fps). The problem with > this is CTS complains if the camera goes faster during the test > than minFrameDuration reported for that resolution. For instance, > > 1080p minFrameDuration: > - Nautilus : 33282000ns > - Soraka : 33147000ns > > Both are less than capped minFrameDuration 1/30 fps (33333333.33ns). > > This patch considers this situation and doesn't cap the > minFrameDuration if the hardware can provide frame durations slightly > better. The tolerance considered is 1% only from the cap. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > On LIMITED level - no regressions were found : 230/231 pass rate > > On FULL level - this fixes the test: > android.hardware.camera2.cts.SurfaceViewPreviewTest#testPreviewFpsRange > --- > src/android/camera_capabilities.cpp | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > index f357902e..c4c26089 100644 > --- a/src/android/camera_capabilities.cpp > +++ b/src/android/camera_capabilities.cpp > @@ -648,7 +648,7 @@ int CameraCapabilities::initializeStreamConfigurations() > int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; > > /* > - * Cap min frame duration to 30 FPS. > + * Cap min frame duration to 30 FPS with 1% tolerance. > * > * 30 frames per second has been validated as the most > * opportune frame rate for quality tuning, and power > @@ -667,8 +667,18 @@ int CameraCapabilities::initializeStreamConfigurations() > * control to be specified for each Request. Defer this > * to the in-development configuration API rework. > */ > - if (minFrameDuration < 1e9 / 30.0) > - minFrameDuration = 1e9 / 30.0; > + int64_t minFrameDurationCap = 1e9 / 30.0; > + if (minFrameDuration < minFrameDurationCap) { > + float tolerance = > + (minFrameDurationCap - minFrameDuration) * 100.0 / minFrameDurationCap; > + > + /* > + * If the tolerance is less than 1%, do not cap > + * the frame duration. > + */ > + if (tolerance > 1.0) > + minFrameDuration = minFrameDurationCap; > + } Could this be simplified to constexpr int64_t kMinFrameDurationCap = 1e9 / 30.0; if (minFrameDuration < kMinFrameDurationCap * 0.99) minFrameDuration = kMinFrameDurationCap; possibly with an appropriate comment. (I would have called the constant kMinFrameDurationLimit but that doesn't matter too much) > > streamConfigurations_.push_back({ > res, androidFormat, minFrameDuration, maxFrameDuration,
diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp index f357902e..c4c26089 100644 --- a/src/android/camera_capabilities.cpp +++ b/src/android/camera_capabilities.cpp @@ -648,7 +648,7 @@ int CameraCapabilities::initializeStreamConfigurations() int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; /* - * Cap min frame duration to 30 FPS. + * Cap min frame duration to 30 FPS with 1% tolerance. * * 30 frames per second has been validated as the most * opportune frame rate for quality tuning, and power @@ -667,8 +667,18 @@ int CameraCapabilities::initializeStreamConfigurations() * control to be specified for each Request. Defer this * to the in-development configuration API rework. */ - if (minFrameDuration < 1e9 / 30.0) - minFrameDuration = 1e9 / 30.0; + int64_t minFrameDurationCap = 1e9 / 30.0; + if (minFrameDuration < minFrameDurationCap) { + float tolerance = + (minFrameDurationCap - minFrameDuration) * 100.0 / minFrameDurationCap; + + /* + * If the tolerance is less than 1%, do not cap + * the frame duration. + */ + if (tolerance > 1.0) + minFrameDuration = minFrameDurationCap; + } streamConfigurations_.push_back({ res, androidFormat, minFrameDuration, maxFrameDuration,
We have some stream resolution which can provide slightly better frame duration than what we cap (i.e. 1/30 fps). The problem with this is CTS complains if the camera goes faster during the test than minFrameDuration reported for that resolution. For instance, 1080p minFrameDuration: - Nautilus : 33282000ns - Soraka : 33147000ns Both are less than capped minFrameDuration 1/30 fps (33333333.33ns). This patch considers this situation and doesn't cap the minFrameDuration if the hardware can provide frame durations slightly better. The tolerance considered is 1% only from the cap. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- On LIMITED level - no regressions were found : 230/231 pass rate On FULL level - this fixes the test: android.hardware.camera2.cts.SurfaceViewPreviewTest#testPreviewFpsRange --- src/android/camera_capabilities.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)