| Message ID | 20210330052521.381550-1-hiroh@chromium.org |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Tue, Mar 30, 2021 at 02:25:18PM +0900, Hirokazu Honda wrote: > Android Camera HAL 3 API used in ChromeOS has a ChromeOS own > extension, for example, crop_rotate_scale_degrees in > camera3_stream. To avoid breaking bisection for platforms using > the API without the extensions, this introduce CHROMEOS macro, > which is defined if and only if android_platform is 'cros'. The main purpose of the macro isn't so much to avoid breaking bisection, but to avoid breaking compilation :-) How about "As those extensions are not available on Android platforms, introduce a CHROMEOS macro that can be used to compile CrOS-specific code conditionally. The macro is defined if and only if android_platform is 'cros'." > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > --- > src/android/meson.build | 6 ++++++ > src/libcamera/meson.build | 6 ++++++ > 2 files changed, 12 insertions(+) > > diff --git a/src/android/meson.build b/src/android/meson.build > index 8e7d07d9..00a3d86b 100644 > --- a/src/android/meson.build > +++ b/src/android/meson.build > @@ -35,6 +35,12 @@ endif > > android_deps += [libyuv_dep] > > +android_cpp_args = [] > + > +if get_option('android_platform') == 'cros' > + android_cpp_args += [ '-DCHROMEOS'] Looking at https://chromium.googlesource.com/chromium/src/build/+/refs/heads/master/config/linux/BUILD.gn, I wonder if we should use OS_CHROMEOS instead of CHROMEOS. > +endif > + > subdir('cros') > > android_hal_sources = files([ > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 815629db..c0443fd2 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -131,10 +131,13 @@ libcamera_deps = [ > > libcamera_link_with = [] > > +libcamera_cpp_args = [] > + You can declare this in src/meson.build, just above libcamera_objects. > if android_enabled > libcamera_sources += android_hal_sources > includes += android_includes > libcamera_link_with += android_camera_metadata > + libcamera_cpp_args += android_cpp_args Then this could be dropped, with and above android_cpp_args replaced with libcamera_cpp_args. > > libcamera_deps += android_deps > endif > @@ -144,10 +147,13 @@ endif > # runtime if the library is running from an installed location by checking > # for the presence or abscence of the dynamic tag. > > +message(libcamera_cpp_args) > + Is this a test leftover ? With those small issues addressed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > libcamera = shared_library('camera', > libcamera_sources, > install : true, > link_with : libcamera_link_with, > + cpp_args : libcamera_cpp_args, > include_directories : includes, > objects : libcamera_objects, > build_rpath : '/',
A HAL client requests the demanded rotation by |crop_rotate_scale_degrees| in camera3_stream in configuration. Libcamera ignores it and doesn't handle the rotation request at all. This patch series still don't support it, but add the validation check to the rotation values and deny if a rotation is requested. Change in v2: - Address Jacopo's comments. Change in v3: - Introduce CHROMEOS define macro suggested by Laurent. Hirokazu Honda (4): android: Define CHROMEOS macro if android_platform=cros android: CameraDevice: Validate crop_rotate_scale_degrees in configuration android: CameraDevice: Log rotation variables in camera3_stream android: CameraDevice: Deny non ROTATION_0 stream configuration src/android/camera_device.cpp | 78 +++++++++++++++++++++++++++++++++++ src/android/meson.build | 6 +++ src/libcamera/meson.build | 6 +++ 3 files changed, 90 insertions(+) -- 2.31.0.291.g576ba9dcdaf-goog