| Message ID | 20210907194107.803730-7-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Tue, Sep 07, 2021 at 09:40:56PM +0200, Jacopo Mondi wrote: > Now that building the list of supported stream configuration requires > applying a configuration to the Camera, re-initialize the camera > controls by applying a configuration generated for the Viewfinder stream > role before building the list of static metadata. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/android/camera_capabilities.cpp | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > index 4e82f12a904e..9bfc7ebc6334 100644 > --- a/src/android/camera_capabilities.cpp > +++ b/src/android/camera_capabilities.cpp > @@ -394,11 +394,14 @@ int CameraCapabilities::initialize(std::shared_ptr<Camera> camera, > } > > ret = initializeStreamConfigurations(); > - camera_->release(); > - if (ret) > + if (ret) { > + camera_->release(); > return ret; > + } > > - return initializeStaticMetadata(); > + ret = initializeStaticMetadata(); > + camera_->release(); > + return ret; > } > > std::vector<Size> > @@ -682,6 +685,19 @@ int CameraCapabilities::initializeStaticMetadata() > return -EINVAL; > } > > + /* > + * Generate and apply a new configuration for the Viewfinder role to > + * collect control limits and properties from a known state. > + */ > + std::unique_ptr<CameraConfiguration> cameraConfig = > + camera_->generateConfiguration({ StreamRole::Viewfinder }); Quite interestingly, we had no guarantee that the camera would be configured with the viewfinder role when reaching this point. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + int ret = camera_->configure(cameraConfig.get()); > + if (ret) { > + LOG(HAL, Error) << "Failed to initialize the camera state"; > + staticMetadata_.reset(); > + return ret; > + } > + > const ControlInfoMap &controlsInfo = camera_->controls(); > const ControlList &properties = camera_->properties(); >
Hi Laurent, On Wed, Oct 06, 2021 at 04:35:16AM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Tue, Sep 07, 2021 at 09:40:56PM +0200, Jacopo Mondi wrote: > > Now that building the list of supported stream configuration requires > > applying a configuration to the Camera, re-initialize the camera > > controls by applying a configuration generated for the Viewfinder stream > > role before building the list of static metadata. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > --- > > src/android/camera_capabilities.cpp | 22 +++++++++++++++++++--- > > 1 file changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > > index 4e82f12a904e..9bfc7ebc6334 100644 > > --- a/src/android/camera_capabilities.cpp > > +++ b/src/android/camera_capabilities.cpp > > @@ -394,11 +394,14 @@ int CameraCapabilities::initialize(std::shared_ptr<Camera> camera, > > } > > > > ret = initializeStreamConfigurations(); > > - camera_->release(); > > - if (ret) > > + if (ret) { > > + camera_->release(); > > return ret; > > + } > > > > - return initializeStaticMetadata(); > > + ret = initializeStaticMetadata(); > > + camera_->release(); > > + return ret; > > } > > > > std::vector<Size> > > @@ -682,6 +685,19 @@ int CameraCapabilities::initializeStaticMetadata() > > return -EINVAL; > > } > > > > + /* > > + * Generate and apply a new configuration for the Viewfinder role to > > + * collect control limits and properties from a known state. > > + */ > > + std::unique_ptr<CameraConfiguration> cameraConfig = > > + camera_->generateConfiguration({ StreamRole::Viewfinder }); > > Quite interestingly, we had no guarantee that the camera would be > configured with the viewfinder role when reaching this point. And, had done by Javier in the other initialize*() functions I should probably check for if (!cameraConfig). > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks j > > > + int ret = camera_->configure(cameraConfig.get()); > > + if (ret) { > > + LOG(HAL, Error) << "Failed to initialize the camera state"; > > + staticMetadata_.reset(); > > + return ret; > > + } > > + > > const ControlInfoMap &controlsInfo = camera_->controls(); > > const ControlList &properties = camera_->properties(); > > > > -- > Regards, > > Laurent Pinchart
diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp index 4e82f12a904e..9bfc7ebc6334 100644 --- a/src/android/camera_capabilities.cpp +++ b/src/android/camera_capabilities.cpp @@ -394,11 +394,14 @@ int CameraCapabilities::initialize(std::shared_ptr<Camera> camera, } ret = initializeStreamConfigurations(); - camera_->release(); - if (ret) + if (ret) { + camera_->release(); return ret; + } - return initializeStaticMetadata(); + ret = initializeStaticMetadata(); + camera_->release(); + return ret; } std::vector<Size> @@ -682,6 +685,19 @@ int CameraCapabilities::initializeStaticMetadata() return -EINVAL; } + /* + * Generate and apply a new configuration for the Viewfinder role to + * collect control limits and properties from a known state. + */ + std::unique_ptr<CameraConfiguration> cameraConfig = + camera_->generateConfiguration({ StreamRole::Viewfinder }); + int ret = camera_->configure(cameraConfig.get()); + if (ret) { + LOG(HAL, Error) << "Failed to initialize the camera state"; + staticMetadata_.reset(); + return ret; + } + const ControlInfoMap &controlsInfo = camera_->controls(); const ControlList &properties = camera_->properties();