<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 16 Feb 2021 at 23:47, 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">Hi Naush,<br>
<br>
On Tue, Feb 16, 2021 at 10:31:36AM +0000, Naushir Patuck wrote:<br>
> Hi,<br>
> <br>
> This series of patches addresses the wasteful usage of embedded data<br>
> streams on sensors that do not supply embedded data. We switch to<br>
> using control lists to pass exposure and gain values from DelayedCtrls<br>
> into the IPA in these cases.<br>
> <br>
> The breakdown of patches is as follows:<br>
> <br>
> Patch 1/4<br>
> Stores the DelayedCtrls::get() provided control list with the bayer<br>
> framebuffer in a queue, and pass this control list to the IPA on a<br>
> RPi::IPA_EVENT_SIGNAL_ISP_PREPARE event.  If there is no embedded<br>
> data, the IPA will simply pull the exposure and gain values from this<br>
> control list.<br>
<br>
A rework of the IPA API has been merged, which causes extensive<br>
conflicts in this patch. I'm sorry for the inconvenience, but could you<br>
rebase the series ?</blockquote><div><br></div><div>Sure no problem.  I'll prepare an updated version shortly.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Not something that needs to be addressed now, but do you think it would<br>
make sense to take an extra step and move parsing of embedded data to<br>
the pipeline handler, always passing controls to the IPA ?<br></blockquote><div><br></div><div>This does seem like a clean approach.  However, I do see a couple of problems:</div><div><br></div><div>- Currently, the CamHelper and parser objects live in the IPA domain, so the pipeline</div><div>handler does not have access to them.</div><div>- There may be cases when the sensor embedded data has vendor/sensor specific</div><div>information stored within it that might be useful to the IPA.  For example, phase detect</div><div>pixel information is something that could be useful for AF algorithms.  In these cases,</div><div>a control list might not be appropriate to pass this information across.</div><div><br></div><div>Ofcourse, things will be different once libcamera has a full implementation of the sensor</div><div>database, and provisions for these could be made.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Patch 2/4<br>
> Remove the MdParserRPi object, it is not used anymore with the above<br>
> change.<br>
> <br>
> Patch 3/4<br>
> This change selectively turns on the Unicam embedded data node on<br>
> sensors that support embedded data.<br>
> <br>
> Patch 4/4<br>
> With the IPA now able to use control lists to extract exposure and<br>
> gain values, we add a flag in the pipeline handler to relax the strict<br>
> bayer <-> embedded data buffer matching routine.  If this flag is set,<br>
> and no match is found, the ipa reverts to the control list values.<br>
> This avoids a few possible frame drops on heavily loaded systems.<br>
> <br>
> Regards,<br>
> Naush<br>
> <br>
> Naushir Patuck (4):<br>
>   pipeline: ipa: raspberrypi: Pass exposure/gain values to IPA though<br>
>     controls<br>
>   ipa: raspberrypi: Remove MdParserRPi<br>
>   pipeline: raspberrypi: Only enabled embedded stream when available<br>
>   pipeline: raspberrypi: Allow either strict or non-strict buffer<br>
>     matching<br>
> <br>
>  src/ipa/raspberrypi/cam_helper.cpp            |   9 +-<br>
>  src/ipa/raspberrypi/cam_helper_imx219.cpp     |   4 +-<br>
>  src/ipa/raspberrypi/cam_helper_ov5647.cpp     |   3 +-<br>
>  src/ipa/raspberrypi/md_parser_rpi.cpp         |  37 ----<br>
>  src/ipa/raspberrypi/md_parser_rpi.hpp         |  32 ---<br>
>  src/ipa/raspberrypi/meson.build               |   1 -<br>
>  src/ipa/raspberrypi/raspberrypi.cpp           | 142 ++++++++-----<br>
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 194 +++++++++++-------<br>
>  8 files changed, 211 insertions(+), 211 deletions(-)<br>
>  delete mode 100644 src/ipa/raspberrypi/md_parser_rpi.cpp<br>
>  delete mode 100644 src/ipa/raspberrypi/md_parser_rpi.hpp<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>