[libcamera-devel] [PATCH 5/5] src: ipa: raspberrypi: Drop the correct number of frames on startup for ov5647

[David Plowman]

Hi Naush

Yes, thanks for the feedback.

On Fri, 4 Dec 2020 at 16:03, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Hi David,
>
> Thank you for your patch.
>
> On Wed, 2 Dec 2020 at 11:53, David Plowman <david.plowman at raspberrypi.com> wrote:
>>
>> The ov5647 delivers two under-exposed frames at startup, even when
>> the exposure and gain are explicitly programmed. The system needs to
>> be told to drop these.
>>
>> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>> ---
>>  src/ipa/raspberrypi/cam_helper_ov5647.cpp | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
>> index dc5d8275..0b841cd1 100644
>> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
>> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
>> @@ -19,6 +19,7 @@ public:
>>         uint32_t GainCode(double gain) const override;
>>         double Gain(uint32_t gain_code) const override;
>>         void GetDelays(int &exposure_delay, int &gain_delay) const override;
>> +       unsigned int HideFramesStartup() const override;
>>         unsigned int HideFramesModeSwitch() const override;
>>         unsigned int MistrustFramesStartup() const override;
>>         unsigned int MistrustFramesModeSwitch() const override;
>> @@ -54,6 +55,15 @@ void CamHelperOv5647::GetDelays(int &exposure_delay, int &gain_delay) const
>>         gain_delay = 2;
>>  }
>>
>> +unsigned int CamHelperOv5647::HideFramesStartup() const
>> +{
>> +       /*
>> +        * On startup, we get a couple of under-exposed frames which
>> +        * we don't want shown.
>> +        */
>> +       return 2;
>> +}
>> +
>
>
> Ah, that might answer part of my question on patch 4/5.  But we still don't account for AWB/LS convergence.  Maybe we don't care?

AWB is a slight issue, I've wondered about that. Generally it's never
as badly "off" as AGC can be, so I ended up not worrying about it. But
the symmetry of giving it the same method and checking both certainly
appeals to me.

ALSC is an interesting one. It, too, is never as badly off as AGC, but
on reflection, if it knew the colour temperature it could produce a
pretty decent table right from the word go. Unfortunately there's no
way to tell it! I wonder whether the AWB could, if it has fixed colour
gains, *deduce* the colour temperature and then set that in the
AwbStatus. Hmmm, that sounds quite cunning, I need to think about it.

Anyway, thanks for the various ideas, I'll look into doing some of
these things (and the name change) and post another set early next
week.

Best regards
David

>
> Regards,
> Naush
>
>
>>
>>  unsigned int CamHelperOv5647::HideFramesModeSwitch() const
>>  {
>>         /*
>> --
>> 2.20.1
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel