<div dir="ltr"><div dir="ltr">Hi Laurent, thank you for the patch.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 15, 2021 at 9:21 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@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">The config_ pointer is reset in all error paths of the<br>
CameraDevice::configureStreams() function, except when<br>
Camera::configure() fails. Fix it by using a local unique pointer to<br>
store the configuration until the end of the function, to avoid similar<br>
issues in the future.<br>
<br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
---<br>
 src/android/camera_device.cpp | 17 ++++++++---------<br>
 1 file changed, 8 insertions(+), 9 deletions(-)<br>
<br>
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp<br>
index f3fd38690e63..f0b5f87bc38f 100644<br>
--- a/src/android/camera_device.cpp<br>
+++ b/src/android/camera_device.cpp<br>
@@ -1716,8 +1716,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)<br>
         * Generate an empty configuration, and construct a StreamConfiguration<br>
         * for each camera3_stream to add to it.<br>
         */<br>
-       config_ = camera_->generateConfiguration();<br>
-       if (!config_) {<br>
+       std::unique_ptr<CameraConfiguration> config = camera_->generateConfiguration();<br>
+       if (!config) {<br>
                LOG(HAL, Error) << "Failed to generate camera configuration";<br>
                return -EINVAL;<br>
        }<br>
@@ -1855,29 +1855,27 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)<br>
<br>
        sortCamera3StreamConfigs(streamConfigs, jpegStream);<br>
        for (const auto &streamConfig : streamConfigs) {<br>
-               config_->addConfiguration(streamConfig.config);<br>
+               config->addConfiguration(streamConfig.config);<br>
<br>
                for (auto &stream : streamConfig.streams) {<br>
                        streams_.emplace_back(this, stream.type, stream.stream,<br>
-                                             config_->size() - 1);<br>
+                                             config->size() - 1); </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                        stream.stream->priv = static_cast<void *>(&streams_.back()); </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                }<br>
        }<br>
<br>
-       switch (config_->validate()) {<br>
+       switch (config->validate()) {<br>
        case CameraConfiguration::Valid:<br>
                break;<br>
        case CameraConfiguration::Adjusted:<br>
                LOG(HAL, Info) << "Camera configuration adjusted";<br>
<br>
-               for (const StreamConfiguration &cfg : *config_)<br>
+               for (const StreamConfiguration &cfg : *config)<br>
                        LOG(HAL, Info) << " - " << cfg.toString();<br>
<br>
-               config_.reset();<br>
                return -EINVAL;<br>
        case CameraConfiguration::Invalid:<br>
                LOG(HAL, Info) << "Camera configuration invalid";<br>
-               config_.reset();<br>
                return -EINVAL;<br>
        }<br>
<br>
@@ -1885,7 +1883,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)<br>
         * Once the CameraConfiguration has been adjusted/validated<br>
         * it can be applied to the camera.<br>
         */<br>
-       int ret = camera_->configure(config_.get());<br>
+       int ret = camera_->configure(config.get());<br>
        if (ret) {<br>
                LOG(HAL, Error) << "Failed to configure camera '"<br>
                                << camera_->id() << "'";<br>
@@ -1905,6 +1903,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)<br>
                }<br>
        }<br>
<br>
+       config_ = std::move(config);<br>
        return 0;<br>
 }<br>
<br></blockquote><div><br></div><div>Shall we also clear streams_ on failure?</div><div><br></div><div>Reviewed-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org">hiroh@chromium.org</a>></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>