[libcamera-devel,1/2] ipa: raspberrypi: AWB: Remove unecessary frame count variable
diff mbox series

Message ID 20210210111743.14374-2-david.plowman@raspberrypi.com
State Accepted
Headers show
Series
  • Raspberry Pi AWB race condition and maintenance
Related show

Commit Message

David Plowman Feb. 10, 2021, 11:17 a.m. UTC
The variable frame_count2_ is not needed as Prepare() and Process()
always run in lock step one after the other.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/rpi/awb.cpp | 6 ++----
 src/ipa/raspberrypi/controller/rpi/awb.hpp | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

Comments

Laurent Pinchart Feb. 10, 2021, 3:44 p.m. UTC | #1
Hi David,

Thank you for the patch.

On Wed, Feb 10, 2021 at 11:17:42AM +0000, David Plowman wrote:
> The variable frame_count2_ is not needed as Prepare() and Process()
> always run in lock step one after the other.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/ipa/raspberrypi/controller/rpi/awb.cpp | 6 ++----
>  src/ipa/raspberrypi/controller/rpi/awb.hpp | 1 -
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> index 791b5108..1c65eda8 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> @@ -153,7 +153,7 @@ void Awb::Read(boost::property_tree::ptree const &params)
>  
>  void Awb::Initialise()
>  {
> -	frame_count2_ = frame_count_ = frame_phase_ = 0;
> +	frame_count_ = frame_phase_ = 0;
>  	// Put something sane into the status that we are filtering towards,
>  	// just in case the first few frames don't have anything meaningful in
>  	// them.
> @@ -288,11 +288,9 @@ void Awb::Process(StatisticsPtr &stats, Metadata *image_metadata)
>  	// Count frames since we last poked the async thread.
>  	if (frame_phase_ < (int)config_.frame_period)
>  		frame_phase_++;
> -	if (frame_count2_ < (int)config_.startup_frames)
> -		frame_count2_++;
>  	LOG(RPiAwb, Debug) << "frame_phase " << frame_phase_;
>  	if (frame_phase_ >= (int)config_.frame_period ||
> -	    frame_count2_ < (int)config_.startup_frames) {
> +	    frame_count_ < (int)config_.startup_frames) {
>  		// Update any settings and any image metadata that we need.
>  		struct LuxStatus lux_status = {};
>  		lux_status.lux = 400; // in case no metadata
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp
> index 1b39ab4b..f113c642 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp
> @@ -130,7 +130,6 @@ private:
>  	// counts up to frame_period before restarting the async thread
>  	int frame_phase_;
>  	int frame_count_; // counts up to startup_frames
> -	int frame_count2_; // counts up to startup_frames for Process method
>  	AwbStatus sync_results_;
>  	AwbStatus prev_sync_results_;
>  	std::string mode_name_;
Naushir Patuck Feb. 10, 2021, 5:14 p.m. UTC | #2
Hi David,

Thank you for your work.

On Wed, 10 Feb 2021 at 11:17, David Plowman <david.plowman@raspberrypi.com>
wrote:

> The variable frame_count2_ is not needed as Prepare() and Process()
> always run in lock step one after the other.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>

Looks good!

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


> ---
>  src/ipa/raspberrypi/controller/rpi/awb.cpp | 6 ++----
>  src/ipa/raspberrypi/controller/rpi/awb.hpp | 1 -
>  2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> index 791b5108..1c65eda8 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> @@ -153,7 +153,7 @@ void Awb::Read(boost::property_tree::ptree const
> &params)
>
>  void Awb::Initialise()
>  {
> -       frame_count2_ = frame_count_ = frame_phase_ = 0;
> +       frame_count_ = frame_phase_ = 0;
>         // Put something sane into the status that we are filtering
> towards,
>         // just in case the first few frames don't have anything
> meaningful in
>         // them.
> @@ -288,11 +288,9 @@ void Awb::Process(StatisticsPtr &stats, Metadata
> *image_metadata)
>         // Count frames since we last poked the async thread.
>         if (frame_phase_ < (int)config_.frame_period)
>                 frame_phase_++;
> -       if (frame_count2_ < (int)config_.startup_frames)
> -               frame_count2_++;
>         LOG(RPiAwb, Debug) << "frame_phase " << frame_phase_;
>         if (frame_phase_ >= (int)config_.frame_period ||
> -           frame_count2_ < (int)config_.startup_frames) {
> +           frame_count_ < (int)config_.startup_frames) {
>                 // Update any settings and any image metadata that we need.
>                 struct LuxStatus lux_status = {};
>                 lux_status.lux = 400; // in case no metadata
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp
> b/src/ipa/raspberrypi/controller/rpi/awb.hpp
> index 1b39ab4b..f113c642 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp
> @@ -130,7 +130,6 @@ private:
>         // counts up to frame_period before restarting the async thread
>         int frame_phase_;
>         int frame_count_; // counts up to startup_frames
> -       int frame_count2_; // counts up to startup_frames for Process
> method
>         AwbStatus sync_results_;
>         AwbStatus prev_sync_results_;
>         std::string mode_name_;
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>

Patch
diff mbox series

diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp
index 791b5108..1c65eda8 100644
--- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
+++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
@@ -153,7 +153,7 @@  void Awb::Read(boost::property_tree::ptree const &params)
 
 void Awb::Initialise()
 {
-	frame_count2_ = frame_count_ = frame_phase_ = 0;
+	frame_count_ = frame_phase_ = 0;
 	// Put something sane into the status that we are filtering towards,
 	// just in case the first few frames don't have anything meaningful in
 	// them.
@@ -288,11 +288,9 @@  void Awb::Process(StatisticsPtr &stats, Metadata *image_metadata)
 	// Count frames since we last poked the async thread.
 	if (frame_phase_ < (int)config_.frame_period)
 		frame_phase_++;
-	if (frame_count2_ < (int)config_.startup_frames)
-		frame_count2_++;
 	LOG(RPiAwb, Debug) << "frame_phase " << frame_phase_;
 	if (frame_phase_ >= (int)config_.frame_period ||
-	    frame_count2_ < (int)config_.startup_frames) {
+	    frame_count_ < (int)config_.startup_frames) {
 		// Update any settings and any image metadata that we need.
 		struct LuxStatus lux_status = {};
 		lux_status.lux = 400; // in case no metadata
diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp
index 1b39ab4b..f113c642 100644
--- a/src/ipa/raspberrypi/controller/rpi/awb.hpp
+++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp
@@ -130,7 +130,6 @@  private:
 	// counts up to frame_period before restarting the async thread
 	int frame_phase_;
 	int frame_count_; // counts up to startup_frames
-	int frame_count2_; // counts up to startup_frames for Process method
 	AwbStatus sync_results_;
 	AwbStatus prev_sync_results_;
 	std::string mode_name_;