[libcamera-devel] [PATCH v2 1/6] libcamera: Move extensible to class

[Kieran Bingham]

Hi Laurent,

On 11/02/2021 20:57, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Feb 11, 2021 at 01:34:39PM +0000, Kieran Bingham wrote:
>> The Extensible concept is a generic Class helper. To prepare for
>> further class helper additions, move the specific extensible
>> implementation and header to a more generic class header and source.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>  include/libcamera/camera.h                  |  2 +-
>>  include/libcamera/camera_manager.h          |  2 +-
>>  include/libcamera/{extensible.h => class.h} |  8 ++++----
>>  include/libcamera/meson.build               |  1 -
>>  src/libcamera/{extensible.cpp => class.cpp} | 11 +++++++----
>>  src/libcamera/meson.build                   |  2 +-
>>  6 files changed, 14 insertions(+), 12 deletions(-)
>>  rename include/libcamera/{extensible.h => class.h} (86%)
>>  rename src/libcamera/{extensible.cpp => class.cpp} (94%)
>>
>> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
>> index f94f859988fe..cff9f46e801b 100644
>> --- a/include/libcamera/camera.h
>> +++ b/include/libcamera/camera.h
>> @@ -12,8 +12,8 @@
>>  #include <stdint.h>
>>  #include <string>
>>  
>> +#include <libcamera/class.h>
>>  #include <libcamera/controls.h>
>> -#include <libcamera/extensible.h>
>>  #include <libcamera/object.h>
>>  #include <libcamera/request.h>
>>  #include <libcamera/signal.h>
>> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
>> index 8c8830e7ff1d..7b8e533fadd6 100644
>> --- a/include/libcamera/camera_manager.h
>> +++ b/include/libcamera/camera_manager.h
>> @@ -12,7 +12,7 @@
>>  #include <sys/types.h>
>>  #include <vector>
>>  
>> -#include <libcamera/extensible.h>
>> +#include <libcamera/class.h>
>>  #include <libcamera/object.h>
>>  #include <libcamera/signal.h>
>>  
>> diff --git a/include/libcamera/extensible.h b/include/libcamera/class.h
>> similarity index 86%
>> rename from include/libcamera/extensible.h
>> rename to include/libcamera/class.h
>> index 3f25a47c5064..2d9b7ebfdb08 100644
>> --- a/include/libcamera/extensible.h
>> +++ b/include/libcamera/class.h
>> @@ -2,10 +2,10 @@
>>  /*
>>   * Copyright (C) 2020, Google Inc.
>>   *
>> - * extensible.h - Utilities to create extensible public classes with stable ABIs
>> + * class.h - Utilities and helpers for public classes
> 
> I'd drop "public" as the macros you will introduce are useful for
> internal classes too.

Ack

> 
>>   */
>> -#ifndef __LIBCAMERA_EXTENSIBLE_H__
>> -#define __LIBCAMERA_EXTENSIBLE_H__
>> +#ifndef __LIBCAMERA_CLASS_H__
>> +#define __LIBCAMERA_CLASS_H__
>>  
>>  #include <memory>
>>  
>> @@ -84,4 +84,4 @@ private:
>>  
>>  } /* namespace libcamera */
>>  
>> -#endif /* __LIBCAMERA_EXTENSIBLE_H__ */
>> +#endif /* __LIBCAMERA_CLASS_H__ */
>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
>> index 13e9eeb6d6ad..aefc60d8c9b4 100644
>> --- a/include/libcamera/meson.build
>> +++ b/include/libcamera/meson.build
>> @@ -7,7 +7,6 @@ libcamera_public_headers = files([
>>      'camera_manager.h',
> 
> Missing class.h ?

Oh indeed! I would have expected that to be caught by checkstyle...

> 
>>      'compiler.h',
>>      'controls.h',
>> -    'extensible.h',
>>      'file_descriptor.h',
>>      'framebuffer_allocator.h',
>>      'geometry.h',
>> diff --git a/src/libcamera/extensible.cpp b/src/libcamera/class.cpp
>> similarity index 94%
>> rename from src/libcamera/extensible.cpp
>> rename to src/libcamera/class.cpp
>> index 1dcb0bf1b12f..8a608edb369b 100644
>> --- a/src/libcamera/extensible.cpp
>> +++ b/src/libcamera/class.cpp
>> @@ -2,14 +2,17 @@
>>  /*
>>   * Copyright (C) 2020, Google Inc.
>>   *
>> - * extensible.cpp - Utilities to create extensible public classes with stable ABIs
>> + * class.cpp - Utilities and helpers for public classes
> 
> Ditto.

Ack

> 
>>   */
>>  
>> -#include <libcamera/extensible.h>
>> +#include <libcamera/class.h>
>>  
>>  /**
>> - * \file extensible.h
>> - * \brief Utilities to create extensible public classes with stable ABIs
>> + * \file class.h
>> + * \brief Utilities to help constructing public class interfaces
> 
> And similarly here.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Thanks

> 
>> + *
>> + * The extensible class can be inheritted to create public classes with stable

s/inheritted/inherited/ too.


>> + * ABIs.
>>   */
>>  
>>  namespace libcamera {
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index 5a4bf0d7ba4f..552893a29dc0 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -9,6 +9,7 @@ libcamera_sources = files([
>>      'camera_controls.cpp',
>>      'camera_manager.cpp',
>>      'camera_sensor.cpp',
>> +    'class.cpp',
>>      'controls.cpp',
>>      'control_serializer.cpp',
>>      'control_validator.cpp',
>> @@ -18,7 +19,6 @@ libcamera_sources = files([
>>      'event_dispatcher.cpp',
>>      'event_dispatcher_poll.cpp',
>>      'event_notifier.cpp',
>> -    'extensible.cpp',
>>      'file.cpp',
>>      'file_descriptor.cpp',
>>      'formats.cpp',
> 

-- 
Regards
--
Kieran