| Message ID | 20210225104217.108792-2-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, On 25/02/2021 10:42, Paul Elder wrote: > Add a cros build option to prepare for adding the symbols required for > the new cros camera API. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > meson_options.txt | 5 +++++ > src/android/meson.build | 5 +++++ > 2 files changed, 10 insertions(+) > > diff --git a/meson_options.txt b/meson_options.txt > index 53f2675e..22efb323 100644 > --- a/meson_options.txt > +++ b/meson_options.txt > @@ -5,6 +5,11 @@ option('android', > value : 'disabled', > description : 'Compile libcamera with Android Camera3 HAL interface') > > +option('cros', > + type : 'boolean', > + value : 'false', > + description : 'Compile libcamera with the cros Camera3 HAL interface (depends on android option)') > + > option('documentation', > type : 'feature', > description : 'Generate the project documentation') > diff --git a/src/android/meson.build b/src/android/meson.build > index 9719c42b..a13ce63b 100644 > --- a/src/android/meson.build > +++ b/src/android/meson.build > @@ -37,6 +37,11 @@ if android_enabled > android_deps += [libyuv_dep] > endif > > +if get_option('cros') and not android_enabled > + warning('cros option enabled but android is not, cros option will be ignored') > +endif > +cros_enabled = get_option('cros') and android_enabled > + What about having cros enable the android option instead? Then ChromeOS platforms only have to select cros=true? > android_hal_sources = files([ > 'camera3_hal.cpp', > 'camera_hal_manager.cpp', >
Hi Paul, Thank you for the patch. On Thu, Feb 25, 2021 at 02:49:15PM +0000, Kieran Bingham wrote: > On 25/02/2021 10:42, Paul Elder wrote: > > Add a cros build option to prepare for adding the symbols required for > > the new cros camera API. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > meson_options.txt | 5 +++++ > > src/android/meson.build | 5 +++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/meson_options.txt b/meson_options.txt > > index 53f2675e..22efb323 100644 > > --- a/meson_options.txt > > +++ b/meson_options.txt > > @@ -5,6 +5,11 @@ option('android', > > value : 'disabled', > > description : 'Compile libcamera with Android Camera3 HAL interface') > > > > +option('cros', > > + type : 'boolean', > > + value : 'false', > > + description : 'Compile libcamera with the cros Camera3 HAL interface (depends on android option)') > > + Jacopo will soon introduce an android_memory_backend option. Would it make sense to merge the two in an android_platform (bikeshedding on the name is possible) ? > > option('documentation', > > type : 'feature', > > description : 'Generate the project documentation') > > diff --git a/src/android/meson.build b/src/android/meson.build > > index 9719c42b..a13ce63b 100644 > > --- a/src/android/meson.build > > +++ b/src/android/meson.build > > @@ -37,6 +37,11 @@ if android_enabled > > android_deps += [libyuv_dep] > > endif > > > > +if get_option('cros') and not android_enabled > > + warning('cros option enabled but android is not, cros option will be ignored') > > +endif > > +cros_enabled = get_option('cros') and android_enabled > > + > > What about having cros enable the android option instead? > > Then ChromeOS platforms only have to select cros=true? Or we could just silently ignore the cros (or android_platform) option when android isn't set, I don't think it would confuse anyone. > > android_hal_sources = files([ > > 'camera3_hal.cpp', > > 'camera_hal_manager.cpp',
Hi Laurent, Paul, On Thu, Feb 25, 2021 at 08:16:04PM +0200, Laurent Pinchart wrote: > Hi Paul, > > Thank you for the patch. > > On Thu, Feb 25, 2021 at 02:49:15PM +0000, Kieran Bingham wrote: > > On 25/02/2021 10:42, Paul Elder wrote: > > > Add a cros build option to prepare for adding the symbols required for > > > the new cros camera API. > > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > --- > > > meson_options.txt | 5 +++++ > > > src/android/meson.build | 5 +++++ > > > 2 files changed, 10 insertions(+) > > > > > > diff --git a/meson_options.txt b/meson_options.txt > > > index 53f2675e..22efb323 100644 > > > --- a/meson_options.txt > > > +++ b/meson_options.txt > > > @@ -5,6 +5,11 @@ option('android', > > > value : 'disabled', > > > description : 'Compile libcamera with Android Camera3 HAL interface') > > > > > > +option('cros', > > > + type : 'boolean', > > > + value : 'false', > > > + description : 'Compile libcamera with the cros Camera3 HAL interface (depends on android option)') > > > + > > Jacopo will soon introduce an android_memory_backend option. Would it > make sense to merge the two in an android_platform (bikeshedding on the > name is possible) ? > I would be glad if this would be possible and I think android_platform is a good compromise. Although I wonder if for the same 'platform' we might have different 'memory backends' depending on, in example, versioning. However I think it's fine for the time being.. Is this change is here to stay or not ? I still wonder if the fix is temporary. > > > option('documentation', > > > type : 'feature', > > > description : 'Generate the project documentation') > > > diff --git a/src/android/meson.build b/src/android/meson.build > > > index 9719c42b..a13ce63b 100644 > > > --- a/src/android/meson.build > > > +++ b/src/android/meson.build > > > @@ -37,6 +37,11 @@ if android_enabled > > > android_deps += [libyuv_dep] > > > endif > > > > > > +if get_option('cros') and not android_enabled > > > + warning('cros option enabled but android is not, cros option will be ignored') > > > +endif > > > +cros_enabled = get_option('cros') and android_enabled > > > + > > > > What about having cros enable the android option instead? > > > > Then ChromeOS platforms only have to select cros=true? > > Or we could just silently ignore the cros (or android_platform) option > when android isn't set, I don't think it would confuse anyone. > > > > android_hal_sources = files([ > > > 'camera3_hal.cpp', > > > 'camera_hal_manager.cpp', > > -- > Regards, > > Laurent Pinchart > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Fri, Feb 26, 2021 at 09:01:25AM +0100, Jacopo Mondi wrote: > On Thu, Feb 25, 2021 at 08:16:04PM +0200, Laurent Pinchart wrote: > > On Thu, Feb 25, 2021 at 02:49:15PM +0000, Kieran Bingham wrote: > > > On 25/02/2021 10:42, Paul Elder wrote: > > > > Add a cros build option to prepare for adding the symbols required for > > > > the new cros camera API. > > > > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > > > meson_options.txt | 5 +++++ > > > > src/android/meson.build | 5 +++++ > > > > 2 files changed, 10 insertions(+) > > > > > > > > diff --git a/meson_options.txt b/meson_options.txt > > > > index 53f2675e..22efb323 100644 > > > > --- a/meson_options.txt > > > > +++ b/meson_options.txt > > > > @@ -5,6 +5,11 @@ option('android', > > > > value : 'disabled', > > > > description : 'Compile libcamera with Android Camera3 HAL interface') > > > > > > > > +option('cros', > > > > + type : 'boolean', > > > > + value : 'false', > > > > + description : 'Compile libcamera with the cros Camera3 HAL interface (depends on android option)') > > > > + > > > > Jacopo will soon introduce an android_memory_backend option. Would it > > make sense to merge the two in an android_platform (bikeshedding on the > > name is possible) ? > > I would be glad if this would be possible and I think android_platform > is a good compromise. Although I wonder if for the same 'platform' we > might have different 'memory backends' depending on, in example, > versioning. However I think it's fine for the time being.. That's true, maybe we'll have to deal with that, time will tell :-) For now it should be fine. > Is this change is here to stay or not ? I still wonder if the fix is > temporary. It's a temporary-to-be-long-term change I think. As far as I understand, making the CrOS-specific HAL API required was an oversight, and this will be fixed. However, it is mandatory to use mojom interfaces, which we don't, but may start using for a CrOS-specific JPEG encoder post-processor (with hardware acceleration) in the future. There's thus no need to overdesign this fix in the short term as it may get reverted, but we may also need it longer term, so any improvement that won't cost us much time is welcome. > > > > option('documentation', > > > > type : 'feature', > > > > description : 'Generate the project documentation') > > > > diff --git a/src/android/meson.build b/src/android/meson.build > > > > index 9719c42b..a13ce63b 100644 > > > > --- a/src/android/meson.build > > > > +++ b/src/android/meson.build > > > > @@ -37,6 +37,11 @@ if android_enabled > > > > android_deps += [libyuv_dep] > > > > endif > > > > > > > > +if get_option('cros') and not android_enabled > > > > + warning('cros option enabled but android is not, cros option will be ignored') > > > > +endif > > > > +cros_enabled = get_option('cros') and android_enabled > > > > + > > > > > > What about having cros enable the android option instead? > > > > > > Then ChromeOS platforms only have to select cros=true? > > > > Or we could just silently ignore the cros (or android_platform) option > > when android isn't set, I don't think it would confuse anyone. > > > > > > android_hal_sources = files([ > > > > 'camera3_hal.cpp', > > > > 'camera_hal_manager.cpp',
diff --git a/meson_options.txt b/meson_options.txt index 53f2675e..22efb323 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -5,6 +5,11 @@ option('android', value : 'disabled', description : 'Compile libcamera with Android Camera3 HAL interface') +option('cros', + type : 'boolean', + value : 'false', + description : 'Compile libcamera with the cros Camera3 HAL interface (depends on android option)') + option('documentation', type : 'feature', description : 'Generate the project documentation') diff --git a/src/android/meson.build b/src/android/meson.build index 9719c42b..a13ce63b 100644 --- a/src/android/meson.build +++ b/src/android/meson.build @@ -37,6 +37,11 @@ if android_enabled android_deps += [libyuv_dep] endif +if get_option('cros') and not android_enabled + warning('cros option enabled but android is not, cros option will be ignored') +endif +cros_enabled = get_option('cros') and android_enabled + android_hal_sources = files([ 'camera3_hal.cpp', 'camera_hal_manager.cpp',
Add a cros build option to prepare for adding the symbols required for the new cros camera API. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- meson_options.txt | 5 +++++ src/android/meson.build | 5 +++++ 2 files changed, 10 insertions(+)