[libcamera-devel] [PATCH] libcamera Android Enhancements

[Laurent Pinchart]

Hi Nicholas,

Thank you for the patch.

On Sat, Oct 22, 2022 at 08:16:13PM -0500, Nicholas Roth via libcamera-devel wrote:
> All,
> 
> For your review, I propose a series of changes that make libcamera more
> accessible for projects and individuals that wish to use it in Android.
> 
> These are the major changes:
> * Adds Makefiles that allow mainline libcamera to build in-place with Android
> distributions, including Waydroid
> * Fixes Bug 159 (see Bugzilla), an LLVM bug that breaks libcamera on Android 11
> and below, which Waydroid uses

I think you meant bug 156.

> * Adds a flag that allows unsigned IPAs to run in-process with a stern warning,
> because openssl and gnutls do not have mainiline Android builds

I don't lije this much. What other options do we have on Android to
verify signatures ?

> * Adds log messages to help people identify and fix issues (e.g., HAL
> configuration file IDs)
> * Adds thread-safety annotations for the raspberry pi IPA to allow it to build
> for Android

Interesting. We compile libcamera with thread-safety annotation, so I
wonder why we haven't caught this. Can you include the error message(s)
you get in the commit message of the corresponding patch in v2 ?

Also, please use include/libcamera/base/thread_annotations.h instead of
defining your own THREAD_ANNOTATION_ATTRIBUTE__ macro.

> * Adds metadata to the rkisp1 IPA that make it compatible with the HAL
> implementation
> * Adds metadata for the PinePhone Pro's image sensors based on mainline Linux
> drivers
> 
> Attachments:
> * patch_logcat.log: Android logcat validating the changes on real HW
> * 0001-libcamera-Android-enhancements.patch: Patch file with the proposed
> changes

As you mentioned in a reply in the thread, this patch should be split.
The above is a good start, I would also split the following changes to
separate patches:

- Usage of std::abs in the Raspberry Pi IPA module
- Removal of std::filesystem usage
- Making libdl optional

The addition of LOG() calls may need to be further split in multiple
patches, I'll comment on that in the review of v2. If there are log
messages that you have added during development that you don't think
should be kept, you can already remove them in the next version.

In v2, could you also please send patches inline, not as attachments ? I
highly recommend usage of git-send-email (with a cover letter) to avoid
lots of potential issues with various mail clients.

> Additional Android Notes:
> On my test setup, my cameras appear correctly in Android settings / diagnostic
> apps as I would expect. However, Android's Java-side camera implementation
> throws an error when I try to get a picture from either camera. I suspect that
> either the rkisp1 IPA needs additional work to play nice with libcamera's
> Android HAL or the Android HAL itself needs work. I plan to ask for feedback
> from other Waydroid users who want to use their (Linux) phones or laptops with
> a camera on Android apps on GitHub to assist with further debugging.
> 
> My full validation setup is documented here: https://docs.google.com/document/d
> /1Sly_VH3w6wFIdE72WXijQegoHZh8IxEnJ9m0hH7GodU/edit

Thank you for the documentation, that will be useful.

> The "[WIP] Camera Support for PinePhone Pro" section contains details specific
> to how I validate libcamera.
> 
> Thanks all, and I look forward to reading your thoughts!

-- 
Regards,

Laurent Pinchart