| Message ID | 20210827120757.110615-15-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, On Fri, Aug 27, 2021 at 02:07:55PM +0200, Jacopo Mondi wrote: > Limit the reported minumum frame duration to 30 FPS. > > The reason to do is to bring the libcamra HAL in par with the Intel > HAL implementation on IPU3 platform, where 30FPS is the frame rate used > to perform quality tuning in the closed-source IPA module and has been > validated as the most efficient rate for the power/performace budget. > > This change bring into the HAL a platform specific constraints, which > might be opportune for most platforms but should rather be configurable > by system integrators. Record that with a \todo entry. > > Also record that, even if we report a lower frame rate, we currently > do not limit what the camera actually produce. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> It looks like this patch is reverted in the next patch...? Paul > --- > src/android/camera_capabilities.cpp | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > index 01cdbcf1395d..6890bfecab29 100644 > --- a/src/android/camera_capabilities.cpp > +++ b/src/android/camera_capabilities.cpp > @@ -632,6 +632,29 @@ int CameraCapabilities::initializeStreamConfigurations() > > int64_t minFrameDuration = frameDurations->second.min().get<int64_t>() * 1000; > int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; > + > + /* > + * Cap min frame duration to 30 FPS. > + * > + * 30 frames per second has been validated as the most > + * opportune frame rate for quality tuning, and power > + * vs performances budget on Intel IPU3. > + * > + * \todo This is a platform-specific decision that needs > + * to be abstracted and delegated to the configuration > + * file. > + * > + * \todo libcamera only allows to control frame duration > + * through the per-request controls::FrameDuration > + * control. If we cap the durations here, we should be > + * capable of configuring the camera to operate at such > + * duration without requiring to have the FrameDuration > + * 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; > + > streamConfigurations_.push_back({ > res, androidFormat, minFrameDuration, maxFrameDuration, > }); > -- > 2.32.0 >
On Wed, Sep 01, 2021 at 05:23:15PM +0900, paul.elder@ideasonboard.com wrote: > Hi Jacopo, > > On Fri, Aug 27, 2021 at 02:07:55PM +0200, Jacopo Mondi wrote: > > Limit the reported minumum frame duration to 30 FPS. > > > > The reason to do is to bring the libcamra HAL in par with the Intel > > HAL implementation on IPU3 platform, where 30FPS is the frame rate used > > to perform quality tuning in the closed-source IPA module and has been > > validated as the most efficient rate for the power/performace budget. > > > > This change bring into the HAL a platform specific constraints, which > > might be opportune for most platforms but should rather be configurable > > by system integrators. Record that with a \todo entry. > > > > Also record that, even if we report a lower frame rate, we currently > > do not limit what the camera actually produce. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > It looks like this patch is reverted in the next patch...? > Yes, the reason is reported in the cover letter > > Paul > > > --- > > src/android/camera_capabilities.cpp | 23 +++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > > index 01cdbcf1395d..6890bfecab29 100644 > > --- a/src/android/camera_capabilities.cpp > > +++ b/src/android/camera_capabilities.cpp > > @@ -632,6 +632,29 @@ int CameraCapabilities::initializeStreamConfigurations() > > > > int64_t minFrameDuration = frameDurations->second.min().get<int64_t>() * 1000; > > int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; > > + > > + /* > > + * Cap min frame duration to 30 FPS. > > + * > > + * 30 frames per second has been validated as the most > > + * opportune frame rate for quality tuning, and power > > + * vs performances budget on Intel IPU3. > > + * > > + * \todo This is a platform-specific decision that needs > > + * to be abstracted and delegated to the configuration > > + * file. > > + * > > + * \todo libcamera only allows to control frame duration > > + * through the per-request controls::FrameDuration > > + * control. If we cap the durations here, we should be > > + * capable of configuring the camera to operate at such > > + * duration without requiring to have the FrameDuration > > + * 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; > > + > > streamConfigurations_.push_back({ > > res, androidFormat, minFrameDuration, maxFrameDuration, > > }); > > -- > > 2.32.0 > >
Hi Jacopo, On Wed, Sep 01, 2021 at 10:29:31AM +0200, Jacopo Mondi wrote: > On Wed, Sep 01, 2021 at 05:23:15PM +0900, paul.elder@ideasonboard.com wrote: > > Hi Jacopo, > > > > On Fri, Aug 27, 2021 at 02:07:55PM +0200, Jacopo Mondi wrote: > > > Limit the reported minumum frame duration to 30 FPS. > > > > > > The reason to do is to bring the libcamra HAL in par with the Intel > > > HAL implementation on IPU3 platform, where 30FPS is the frame rate used > > > to perform quality tuning in the closed-source IPA module and has been > > > validated as the most efficient rate for the power/performace budget. > > > > > > This change bring into the HAL a platform specific constraints, which > > > might be opportune for most platforms but should rather be configurable > > > by system integrators. Record that with a \todo entry. > > > > > > Also record that, even if we report a lower frame rate, we currently > > > do not limit what the camera actually produce. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > > It looks like this patch is reverted in the next patch...? > > > > Yes, the reason is reported in the cover letter Oh oops, I read that yesterday and forgot about it :p I vote for the IPA solution. Paul > > > > > --- > > > src/android/camera_capabilities.cpp | 23 +++++++++++++++++++++++ > > > 1 file changed, 23 insertions(+) > > > > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > > > index 01cdbcf1395d..6890bfecab29 100644 > > > --- a/src/android/camera_capabilities.cpp > > > +++ b/src/android/camera_capabilities.cpp > > > @@ -632,6 +632,29 @@ int CameraCapabilities::initializeStreamConfigurations() > > > > > > int64_t minFrameDuration = frameDurations->second.min().get<int64_t>() * 1000; > > > int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; > > > + > > > + /* > > > + * Cap min frame duration to 30 FPS. > > > + * > > > + * 30 frames per second has been validated as the most > > > + * opportune frame rate for quality tuning, and power > > > + * vs performances budget on Intel IPU3. > > > + * > > > + * \todo This is a platform-specific decision that needs > > > + * to be abstracted and delegated to the configuration > > > + * file. > > > + * > > > + * \todo libcamera only allows to control frame duration > > > + * through the per-request controls::FrameDuration > > > + * control. If we cap the durations here, we should be > > > + * capable of configuring the camera to operate at such > > > + * duration without requiring to have the FrameDuration > > > + * 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; > > > + > > > streamConfigurations_.push_back({ > > > res, androidFormat, minFrameDuration, maxFrameDuration, > > > }); > > > -- > > > 2.32.0 > > >
diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp index 01cdbcf1395d..6890bfecab29 100644 --- a/src/android/camera_capabilities.cpp +++ b/src/android/camera_capabilities.cpp @@ -632,6 +632,29 @@ int CameraCapabilities::initializeStreamConfigurations() int64_t minFrameDuration = frameDurations->second.min().get<int64_t>() * 1000; int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000; + + /* + * Cap min frame duration to 30 FPS. + * + * 30 frames per second has been validated as the most + * opportune frame rate for quality tuning, and power + * vs performances budget on Intel IPU3. + * + * \todo This is a platform-specific decision that needs + * to be abstracted and delegated to the configuration + * file. + * + * \todo libcamera only allows to control frame duration + * through the per-request controls::FrameDuration + * control. If we cap the durations here, we should be + * capable of configuring the camera to operate at such + * duration without requiring to have the FrameDuration + * 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; + streamConfigurations_.push_back({ res, androidFormat, minFrameDuration, maxFrameDuration, });
Limit the reported minumum frame duration to 30 FPS. The reason to do is to bring the libcamra HAL in par with the Intel HAL implementation on IPU3 platform, where 30FPS is the frame rate used to perform quality tuning in the closed-source IPA module and has been validated as the most efficient rate for the power/performace budget. This change bring into the HAL a platform specific constraints, which might be opportune for most platforms but should rather be configurable by system integrators. Record that with a \todo entry. Also record that, even if we report a lower frame rate, we currently do not limit what the camera actually produce. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/android/camera_capabilities.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)