| Message ID | 20200731140801.13253-3-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for the patch. On Fri, Jul 31, 2020 at 03:07:59PM +0100, David Plowman wrote: > Fix up a few locations where we call notify_one() with the lock > held. In particular, restartAsync does not need to be called with the > lock held for its entire duration. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp > index 76e2f04..12359dc 100644 > --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp > @@ -32,8 +32,8 @@ Alsc::~Alsc() > { > std::lock_guard<std::mutex> lock(mutex_); > async_abort_ = true; > - async_signal_.notify_one(); > } > + async_signal_.notify_one(); > async_thread_.join(); > } > > @@ -268,8 +268,11 @@ void Alsc::restartAsync(StatisticsPtr &stats, Metadata *image_metadata) > frame_phase_ = 0; > // copy the camera mode so it won't change during the calculations > async_camera_mode_ = camera_mode_; > - async_start_ = true; > async_started_ = true; > + { > + std::lock_guard<std::mutex> lock(mutex_); > + async_start_ = true; > + } > async_signal_.notify_one(); > } > > @@ -315,7 +318,6 @@ void Alsc::Process(StatisticsPtr &stats, Metadata *image_metadata) > RPI_LOG("Alsc: frame_phase " << frame_phase_); > if (frame_phase_ >= (int)config_.frame_period || > frame_count2_ < (int)config_.startup_frames) { > - std::unique_lock<std::mutex> lock(mutex_); > if (async_started_ == false) { > RPI_LOG("ALSC thread starting"); > restartAsync(stats, image_metadata); > @@ -339,8 +341,8 @@ void Alsc::asyncFunc() > { > std::lock_guard<std::mutex> lock(mutex_); > async_finished_ = true; > - sync_signal_.notify_one(); > } > + sync_signal_.notify_one(); > } > } >
diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp index 76e2f04..12359dc 100644 --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp @@ -32,8 +32,8 @@ Alsc::~Alsc() { std::lock_guard<std::mutex> lock(mutex_); async_abort_ = true; - async_signal_.notify_one(); } + async_signal_.notify_one(); async_thread_.join(); } @@ -268,8 +268,11 @@ void Alsc::restartAsync(StatisticsPtr &stats, Metadata *image_metadata) frame_phase_ = 0; // copy the camera mode so it won't change during the calculations async_camera_mode_ = camera_mode_; - async_start_ = true; async_started_ = true; + { + std::lock_guard<std::mutex> lock(mutex_); + async_start_ = true; + } async_signal_.notify_one(); } @@ -315,7 +318,6 @@ void Alsc::Process(StatisticsPtr &stats, Metadata *image_metadata) RPI_LOG("Alsc: frame_phase " << frame_phase_); if (frame_phase_ >= (int)config_.frame_period || frame_count2_ < (int)config_.startup_frames) { - std::unique_lock<std::mutex> lock(mutex_); if (async_started_ == false) { RPI_LOG("ALSC thread starting"); restartAsync(stats, image_metadata); @@ -339,8 +341,8 @@ void Alsc::asyncFunc() { std::lock_guard<std::mutex> lock(mutex_); async_finished_ = true; - sync_signal_.notify_one(); } + sync_signal_.notify_one(); } }
Fix up a few locations where we call notify_one() with the lock held. In particular, restartAsync does not need to be called with the lock held for its entire duration. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- src/ipa/raspberrypi/controller/rpi/alsc.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)