[libcamera-devel] [PATCH 3/5] include: android: Add Android headers from Cros

[Laurent Pinchart]

On Mon, Aug 05, 2019 at 02:45:22PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Thu, Aug 01, 2019 at 05:54:18PM +0200, Jacopo Mondi wrote:
> > Copy the Android Camera3 HAL headers from the ChromiumOS build system
> > and define a new inclusion directive in the meson build system for them.
> > 
> > The header files have been copied from:
> > https://chromium.googlesource.com/chromiumos/platform2
> > at revision ceb477360a8012ee38e80746258f4828aad6b4c7
> > and provide:
> > 
> > 1) Android CameraHAL3 HAL headers in include/android/hardware/
> > 2) The Android system headers in include/android/system/
> > 3) The Android camera metadata headers in include/android/metadata/
> > 
> > The original path in the Cros platform2/ repository is, respectively:
> > camera/android/header_files/include/hardware
> > camera/android/header_files/include/system
> > camera/android/libcamera_metadata/include/
> 
> We may want to copy the files from AOSP instead, but for now this will
> do.
> 
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  .../libhardware/include/hardware/camera3.h    | 3093 +++++++++++++++++
> >  .../include/hardware/camera_common.h          |  916 +++++
> >  .../libhardware/include/hardware/fb.h         |  173 +
> >  .../libhardware/include/hardware/gralloc.h    |  384 ++
> >  .../libhardware/include/hardware/hardware.h   |  238 ++
> >  .../android/metadata/camera_metadata_hidden.h |  100 +
> >  .../android/metadata/system/camera_metadata.h |  580 ++++
> >  .../metadata/system/camera_metadata_tags.h    | 1005 ++++++
> >  .../metadata/system/camera_vendor_tags.h      |  158 +
> >  .../android/system/core/include/android/log.h |  144 +
> >  .../system/core/include/cutils/compiler.h     |   44 +
> >  .../core/include/cutils/native_handle.h       |   69 +
> >  .../system/core/include/system/camera.h       |  298 ++
> >  .../system/core/include/system/graphics.h     |  763 ++++
> >  .../system/core/include/system/window.h       |  954 +++++
> >  meson.build                                   |    5 +
> >  16 files changed, 8924 insertions(+)
> >  create mode 100644 include/android/hardware/libhardware/include/hardware/camera3.h
> >  create mode 100644 include/android/hardware/libhardware/include/hardware/camera_common.h
> >  create mode 100644 include/android/hardware/libhardware/include/hardware/fb.h
> >  create mode 100644 include/android/hardware/libhardware/include/hardware/gralloc.h
> >  create mode 100644 include/android/hardware/libhardware/include/hardware/hardware.h
> >  create mode 100644 include/android/metadata/camera_metadata_hidden.h
> >  create mode 100644 include/android/metadata/system/camera_metadata.h
> >  create mode 100644 include/android/metadata/system/camera_metadata_tags.h
> >  create mode 100644 include/android/metadata/system/camera_vendor_tags.h
> >  create mode 100644 include/android/system/core/include/android/log.h
> >  create mode 100644 include/android/system/core/include/cutils/compiler.h
> >  create mode 100644 include/android/system/core/include/cutils/native_handle.h
> >  create mode 100644 include/android/system/core/include/system/camera.h
> >  create mode 100644 include/android/system/core/include/system/graphics.h
> >  create mode 100644 include/android/system/core/include/system/window.h
> > 
> > diff --git a/include/android/hardware/libhardware/include/hardware/camera3.h b/include/android/hardware/libhardware/include/hardware/camera3.h
> > new file mode 100644
> > index 000000000000..49664dc3ceb6
> > --- /dev/null
> > +++ b/include/android/hardware/libhardware/include/hardware/camera3.h
> > @@ -0,0 +1,3093 @@
> 
> 
> Could you add SPDX headers in a separate patch ? You can keep the
> copyright and license text below unchanged, but every file should have
> an SPDX header. This comment applieds to all the include/android/ files.
> 
> The license should also be added to licenses/.
> 
> > +/*
> > + * Copyright (C) 2013 The Android Open Source Project
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at
> > + *
> > + *      http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> 
> [snip]
> 
> > diff --git a/meson.build b/meson.build
> > index 13d0605f903c..2575d0abb334 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -51,6 +51,11 @@ add_project_arguments(cpp_arguments, language : 'cpp')
> >  add_project_link_arguments(cpp_arguments, language : 'cpp')
> >  
> >  libcamera_includes = include_directories('include')
> > +android_includes = ([
> > +    include_directories('include/android/hardware/libhardware/include/'),
> > +    include_directories('include/android/system/core/include'),
> > +    include_directories('include/android/metadata/'),
> > +])
> 
> I would move this to src/android/meson.build (and thus to patch 3/5) as
> I don't think it belongs to the top-level meson.build. Don't forget to
> remove the corresponding sentence from the commit message of this patch.

And now that I wrote this I of course realise it's not correct, as the
include directory is at the top level. How about moving it to
include/android/meson.build ?

> >  
> >  subdir('include')
> >  subdir('src')

-- 
Regards,

Laurent Pinchart