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

[Laurent Pinchart]

Hi Niklas,

On Sat, Jul 24, 2021 at 09:01:03AM +0200, Niklas Söderlund wrote:
> On 2021-07-23 07:00:25 +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
> 
> Unrelated change ;-) With or without this commented in the commit 
> message,

With the addition of internal/camera.h, "camera.h" became ambiguous, and
doxygen started warning about it. I'll mention it in the commit message.

> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> 
> >   * \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