[libcamera-devel,02/22] ipu3: ipa: Report effective sensor controls with statistics to IPA
diff mbox series

Message ID 20211108131350.130665-3-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • IPA: IPU3: Introduce per-frame controls
Related show

Commit Message

Jean-Michel Hautbois Nov. 8, 2021, 1:13 p.m. UTC 
From: Han-Lin Chen <hanlinchen@chromium.org>

The Intel close sourced IPA requires the effective controls applied to
the sensor when the statistics are generated. Report effective sensor controls
with the stastistics to IPA.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
[Jean-Michel: Reword s/stastistics/statistics]
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/frames.h | 3 +++
 src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++
 2 files changed, 8 insertions(+)

Comments

Kieran Bingham Nov. 8, 2021, 1:48 p.m. UTC | #1 
Quoting Jean-Michel Hautbois (2021-11-08 13:13:30)
> From: Han-Lin Chen <hanlinchen@chromium.org>
> 
> The Intel close sourced IPA requires the effective controls applied to
> the sensor when the statistics are generated. Report effective sensor controls
> with the stastistics to IPA.
> 
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> [Jean-Michel: Reword s/stastistics/statistics]

You missed one in the commit message ;-)

> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/frames.h | 3 +++
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
> index 3ef7e445..a897e307 100644
> --- a/src/libcamera/pipeline/ipu3/frames.h
> +++ b/src/libcamera/pipeline/ipu3/frames.h
> @@ -12,6 +12,7 @@
>  #include <queue>
>  #include <vector>
>  
> +#include <libcamera/controls.h>

I suspect checkstyle would suggest a separate group here.

>  #include <libcamera/base/signal.h>
>  
>  namespace libcamera {
> @@ -34,6 +35,8 @@ public:
>                 FrameBuffer *paramBuffer;
>                 FrameBuffer *statBuffer;
>  
> +               ControlList effectiveSensorControls;
> +
>                 bool paramDequeued;
>                 bool metadataProcessed;
>         };
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 8816efc5..6a7f5b9a 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -667,6 +667,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>                 return ret;
>         }
>  
> +       data->delayedCtrls_->reset();
> +

Did I see a commit titled that moves this later?

>         return updateControls(data);
>  }
>  
> @@ -1363,6 +1365,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>         request->metadata().set(controls::SensorTimestamp,
>                                 buffer->metadata().timestamp);
>  
> +       info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);
> +
>         if (request->findBuffer(&rawStream_))
>                 pipe()->completeBuffer(request, buffer);
>  
> @@ -1419,6 +1423,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>         ev.frame = info->id;
>         ev.bufferId = info->statBuffer->cookie();
>         ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp);
> +       ev.sensorControls = info->effectiveSensorControls;
>         ipa_->processEvent(ev);
>  }
>  
> -- 
> 2.32.0
>
Jean-Michel Hautbois Nov. 9, 2021, 1:26 p.m. UTC | #2 
Hi Kieran,

On 08/11/2021 14:48, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-11-08 13:13:30)
>> From: Han-Lin Chen <hanlinchen@chromium.org>
>>
>> The Intel close sourced IPA requires the effective controls applied to
>> the sensor when the statistics are generated. Report effective sensor controls
>> with the stastistics to IPA.
>>
>> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
>> [Jean-Michel: Reword s/stastistics/statistics]
> 
> You missed one in the commit message ;-)

Thanks :-)

> 
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
>> ---
>>   src/libcamera/pipeline/ipu3/frames.h | 3 +++
>>   src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
>> index 3ef7e445..a897e307 100644
>> --- a/src/libcamera/pipeline/ipu3/frames.h
>> +++ b/src/libcamera/pipeline/ipu3/frames.h
>> @@ -12,6 +12,7 @@
>>   #include <queue>
>>   #include <vector>
>>   
>> +#include <libcamera/controls.h>
> 
> I suspect checkstyle would suggest a separate group here.

Not on my side...

> 
>>   #include <libcamera/base/signal.h>
>>   
>>   namespace libcamera {
>> @@ -34,6 +35,8 @@ public:
>>                  FrameBuffer *paramBuffer;
>>                  FrameBuffer *statBuffer;
>>   
>> +               ControlList effectiveSensorControls;
>> +
>>                  bool paramDequeued;
>>                  bool metadataProcessed;
>>          };
>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> index 8816efc5..6a7f5b9a 100644
>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> @@ -667,6 +667,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>>                  return ret;
>>          }
>>   
>> +       data->delayedCtrls_->reset();
>> +
> 
> Did I see a commit titled that moves this later?

Yes, I will squash both ;-)
> 
>>          return updateControls(data);
>>   }
>>   
>> @@ -1363,6 +1365,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>>          request->metadata().set(controls::SensorTimestamp,
>>                                  buffer->metadata().timestamp);
>>   
>> +       info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);
>> +
>>          if (request->findBuffer(&rawStream_))
>>                  pipe()->completeBuffer(request, buffer);
>>   
>> @@ -1419,6 +1423,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>>          ev.frame = info->id;
>>          ev.bufferId = info->statBuffer->cookie();
>>          ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp);
>> +       ev.sensorControls = info->effectiveSensorControls;
>>          ipa_->processEvent(ev);
>>   }
>>   
>> -- 
>> 2.32.0
>>
Kieran Bingham Nov. 10, 2021, 10:10 a.m. UTC | #3 
Quoting Jean-Michel Hautbois (2021-11-09 13:26:26)
> Hi Kieran,
> 
> On 08/11/2021 14:48, Kieran Bingham wrote:
> > Quoting Jean-Michel Hautbois (2021-11-08 13:13:30)
> >> From: Han-Lin Chen <hanlinchen@chromium.org>
> >>
> >> The Intel close sourced IPA requires the effective controls applied to
> >> the sensor when the statistics are generated. Report effective sensor controls
> >> with the stastistics to IPA.
> >>
> >> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> >> [Jean-Michel: Reword s/stastistics/statistics]
> > 
> > You missed one in the commit message ;-)
> 
> Thanks :-)
> 
> > 
> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> >> ---
> >>   src/libcamera/pipeline/ipu3/frames.h | 3 +++
> >>   src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++
> >>   2 files changed, 8 insertions(+)
> >>
> >> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
> >> index 3ef7e445..a897e307 100644
> >> --- a/src/libcamera/pipeline/ipu3/frames.h
> >> +++ b/src/libcamera/pipeline/ipu3/frames.h
> >> @@ -12,6 +12,7 @@
> >>   #include <queue>
> >>   #include <vector>
> >>   
> >> +#include <libcamera/controls.h>
> > 
> > I suspect checkstyle would suggest a separate group here.
> 
> Not on my side...

Seems other patches I've seen lately haven't been showing up in
checkstyle either, but running clang-format directly highlights:


--- src/libcamera/pipeline/ipu3/frames.h
+++ src/libcamera/pipeline/ipu3/frames.h.clang
@@ -9,12 +9,14 @@

 #include <map>
 #include <memory>
 #include <vector>

-#include <libcamera/controls.h>
 #include <libcamera/base/signal.h>

+#include <libcamera/controls.h>
+
 namespace libcamera {

 class FrameBuffer;

(Ignore the '<queue>' move if you see that locally, that's a known
false-positive)

> 
> > 
> >>   #include <libcamera/base/signal.h>
> >>   
> >>   namespace libcamera {
> >> @@ -34,6 +35,8 @@ public:
> >>                  FrameBuffer *paramBuffer;
> >>                  FrameBuffer *statBuffer;
> >>   
> >> +               ControlList effectiveSensorControls;
> >> +
> >>                  bool paramDequeued;
> >>                  bool metadataProcessed;
> >>          };
> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> index 8816efc5..6a7f5b9a 100644
> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> @@ -667,6 +667,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >>                  return ret;
> >>          }
> >>   
> >> +       data->delayedCtrls_->reset();
> >> +
> > 
> > Did I see a commit titled that moves this later?
> 
> Yes, I will squash both ;-)
> > 
> >>          return updateControls(data);
> >>   }
> >>   
> >> @@ -1363,6 +1365,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> >>          request->metadata().set(controls::SensorTimestamp,
> >>                                  buffer->metadata().timestamp);
> >>   
> >> +       info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);
> >> +
> >>          if (request->findBuffer(&rawStream_))
> >>                  pipe()->completeBuffer(request, buffer);
> >>   
> >> @@ -1419,6 +1423,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
> >>          ev.frame = info->id;
> >>          ev.bufferId = info->statBuffer->cookie();
> >>          ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp);
> >> +       ev.sensorControls = info->effectiveSensorControls;
> >>          ipa_->processEvent(ev);
> >>   }
> >>   
> >> -- 
> >> 2.32.0
> >>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
index 3ef7e445..a897e307 100644
--- a/src/libcamera/pipeline/ipu3/frames.h
+++ b/src/libcamera/pipeline/ipu3/frames.h
@@ -12,6 +12,7 @@ 
 #include <queue>
 #include <vector>
 
+#include <libcamera/controls.h>
 #include <libcamera/base/signal.h>
 
 namespace libcamera {
@@ -34,6 +35,8 @@  public:
 		FrameBuffer *paramBuffer;
 		FrameBuffer *statBuffer;
 
+		ControlList effectiveSensorControls;
+
 		bool paramDequeued;
 		bool metadataProcessed;
 	};
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 8816efc5..6a7f5b9a 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -667,6 +667,8 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 		return ret;
 	}
 
+	data->delayedCtrls_->reset();
+
 	return updateControls(data);
 }
 
@@ -1363,6 +1365,8 @@  void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
 	request->metadata().set(controls::SensorTimestamp,
 				buffer->metadata().timestamp);
 
+	info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);
+
 	if (request->findBuffer(&rawStream_))
 		pipe()->completeBuffer(request, buffer);
 
@@ -1419,6 +1423,7 @@  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
 	ev.frame = info->id;
 	ev.bufferId = info->statBuffer->cookie();
 	ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp);
+	ev.sensorControls = info->effectiveSensorControls;
 	ipa_->processEvent(ev);
 }