[libcamera-devel] [RFC PATCH 06/17] libcamera: camera: Move Camera::Private to header file

[Jacopo Mondi]

Hi Laurent,

On Fri, Jul 23, 2021 at 07:00:25AM +0300, Laurent Pinchart wrote:
> The Camera::Private class is defined in camera.cpp. To prepare for
> allowing it to be subclassed by pipeline handlers, move it to a new
> internal/camera.h header.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/internal/camera.h    | 63 ++++++++++++++++++++++++++
>  include/libcamera/internal/meson.build |  1 +
>  src/libcamera/camera.cpp               | 40 +---------------
>  3 files changed, 66 insertions(+), 38 deletions(-)
>  create mode 100644 include/libcamera/internal/camera.h
>
> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
> new file mode 100644
> index 000000000000..9ef5d8ae98a6
> --- /dev/null
> +++ b/include/libcamera/internal/camera.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * camera.h - Camera private data
> + */
> +#ifndef __LIBCAMERA_INTERNAL_CAMERA_H__
> +#define __LIBCAMERA_INTERNAL_CAMERA_H__
> +
> +#include <atomic>
> +#include <memory>
> +#include <set>
> +#include <string>
> +
> +#include <libcamera/base/class.h>
> +
> +#include <libcamera/camera.h>
> +
> +namespace libcamera {
> +
> +class PipelineHandler;
> +class Stream;
> +
> +class Camera::Private : public Extensible::Private
> +{
> +	LIBCAMERA_DECLARE_PUBLIC(Camera)
> +
> +public:
> +	enum State {
> +		CameraAvailable,
> +		CameraAcquired,
> +		CameraConfigured,
> +		CameraStopping,
> +		CameraRunning,
> +	};
> +
> +	Private(PipelineHandler *pipe, const std::string &id,
> +		const std::set<Stream *> &streams);
> +	~Private();
> +
> +	bool isRunning() const;
> +	int isAccessAllowed(State state, bool allowDisconnected = false,
> +			    const char *from = __builtin_FUNCTION()) const;
> +	int isAccessAllowed(State low, State high,
> +			    bool allowDisconnected = false,
> +			    const char *from = __builtin_FUNCTION()) const;
> +
> +	void disconnect();
> +	void setState(State state);
> +
> +	std::shared_ptr<PipelineHandler> pipe_;
> +	std::string id_;
> +	std::set<Stream *> streams_;
> +	std::set<const Stream *> activeStreams_;
> +
> +private:
> +	bool disconnected_;
> +	std::atomic<State> state_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_CAMERA_H__ */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 6d4eb7ed3df6..dac1a2d36fa8 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -12,6 +12,7 @@ libcamera_tracepoint_header = custom_target(
>  libcamera_internal_headers = files([
>      'bayer_format.h',
>      'byte_stream_buffer.h',
> +    'camera.h',
>      'camera_controls.h',
>      'camera_sensor.h',
>      'camera_sensor_properties.h',
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index c126b49290ce..4b5bc891fc37 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -18,10 +18,11 @@
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>
> +#include "libcamera/internal/camera.h"
>  #include "libcamera/internal/pipeline_handler.h"
>
>  /**
> - * \file camera.h
> + * \file libcamera/camera.h

Documentation will be grouped in a single page for both public and
private, I assume this is intentional ?

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j

>   * \brief Camera device handling
>   *
>   * \page camera-model Camera Model
> @@ -331,43 +332,6 @@ std::size_t CameraConfiguration::size() const
>   * \brief The vector of stream configurations
>   */
>
> -class Camera::Private : public Extensible::Private
> -{
> -	LIBCAMERA_DECLARE_PUBLIC(Camera)
> -
> -public:
> -	enum State {
> -		CameraAvailable,
> -		CameraAcquired,
> -		CameraConfigured,
> -		CameraStopping,
> -		CameraRunning,
> -	};
> -
> -	Private(PipelineHandler *pipe, const std::string &id,
> -		const std::set<Stream *> &streams);
> -	~Private();
> -
> -	bool isRunning() const;
> -	int isAccessAllowed(State state, bool allowDisconnected = false,
> -			    const char *from = __builtin_FUNCTION()) const;
> -	int isAccessAllowed(State low, State high,
> -			    bool allowDisconnected = false,
> -			    const char *from = __builtin_FUNCTION()) const;
> -
> -	void disconnect();
> -	void setState(State state);
> -
> -	std::shared_ptr<PipelineHandler> pipe_;
> -	std::string id_;
> -	std::set<Stream *> streams_;
> -	std::set<const Stream *> activeStreams_;
> -
> -private:
> -	bool disconnected_;
> -	std::atomic<State> state_;
> -};
> -
>  Camera::Private::Private(PipelineHandler *pipe,
>  			 const std::string &id,
>  			 const std::set<Stream *> &streams)
> --
> Regards,
>
> Laurent Pinchart
>