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

[Laurent Pinchart]

Hi Jacopo,

On Thu, Jul 29, 2021 at 10:44:13PM +0200, Jacopo Mondi wrote:
> 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 ?

For now, yes. At some point in the future I'd like to generate two sets
of documentation, one for the public API only, and one for the internal
API (which should at least have links to the public API where
applicable, but may combine both the public and private APIs, not sure
yet).

> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> >   * \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