[libcamera-devel] [PATCH 00/38] IPA isolation implementation

[Laurent Pinchart]

Hi Han-lin,

On Wed, Sep 30, 2020 at 05:43:23PM +0800, Han-lin Chen wrote:
> Hi Paul and Laurent,
> 
> Some reflection from ChromeOS integration's perspective.
> Currently, ChromeOS separates HAL and 3A into distinct upstart services.
> The upstart will launch each one in minijail with different settings.
> The reason is that we'd like the 3A component to have more restrictions than
> the HAL process, since it usually contains a closed sourced library.
> The upstart will launch them separately and they will try to connect to each
> other by a specific named socket.
> On the socket connection, they communicate by a predefined mojo interface.
> For vendors, we require the close sourced library to expose symbols identical
> to the interface.
> 
> In my opinion, ChromeOS doesn't need to stick to mojo communication, and we can
> apply this solution.
> But we hope to maintain the two-service structure, and thus something I can
> think of might influence.
> I don't have a clear opinion for them, but I'd like to add them here for
> discussion.
>
> 1. Since the 3A component is launched by an independent service and in our case
> it should be ipa_proxy_worker. ChromeOS needs a way to control the libcamera
> not to fork the ipa_proxy_worker, but to connect to a predefined named socket.
> Maybe by specific build args for ChromeOS or an extended API to its hal
> adaptor.
> 2. Because 3A is close-sourced, we expected it to crash for unknown reasons. In
> this case, the upstart service will respawn the 3A process, and the HAL process
> should be able to detect it and try to reconnect, assuming 3A is the server
> side of the socket.

The 3A process will then lose its state. Does the HAL detect this and
reinitialize it ?

> 3. Sometimes the HAL process might crash too :). The best case is that the
> respawned HAL can reconnect to 3A again. Or for simplicity, 3A can exit() on
> disconnection and assume upstart will restart everything.
> 4. This one may not be an actual problem but just a concern. For vendors, we
> require the close sourced library to expose the same symbols defined by us.
> The definition: https://chromium.googlesource.com/chromiumos/platform2/+/refs/
> heads/master/camera/include/cros-camera/camera_algorithm.h
> For existing ChromeOS products, the binaries are deployed and following the
> ChromeOS API. On the other hand, libcamera has more freedom on defining the 3A
> API. In the ideal situation, we could discuss with every vendor to support
> libcamera's API too, but I have to admit that it depends on the vendor, and is
> not always possible. If this happens, and the libcamera IPA API can be easily
> translated to ChromeOS IPA API, we can add an intermediate layer to adapt the
> existing binaries, and avoid the problem.

We're planning to have a look at this for IPU3, and write an adaptation
between the Chrome OS interface and libcamera. Given that the 3A binary
from Intel is closed-source it will be challenging to debug issues, but
we'll do our best :-)

> For other platforms we don't have products, we don't need to worry about it,
> but It may affect rockchip, ipu3/ipu6, Mediatek and maybe Qualcomm devices.

I agree with you overal. To shed a bit of light on the rationale for the
design, we are envisioning different IPC backends for different
platforms. We don't want at this point to implement a camera algorithm
daemon specific to libcamera (mostly because it will take time to do
so), so we have implemented a solution that isolates the IPA module in a
process dynamically forked at runtime. That process is not currently
sandboxed, but we're considering using minijail for this purpose. Once
the initial implementation will be merged, with a functional end-to-end
IPC mechanism for IPA modules, we will consider extending this with
other options, such as an IPC mechanism that will communicate with a
daemon instead of forking a process.

Would this make sense for you ?

> Thanks for the implementation Paul, the idea to reuse mojom parser is so
> clever.

-- 
Regards,

Laurent Pinchart