[libcamera-devel,12/20] libcamera: pipeline: simple: Document the pipeline handler design
diff mbox series

Message ID 20210131224702.8838-13-laurent.pinchart@ideasonboard.com
State Accepted
Delegated to: Laurent Pinchart
Headers show
Series
  • [libcamera-devel,01/20] libcamera: pipeline: simple: Manage converter with std::unique_ptr<>
Related show

Commit Message

Laurent Pinchart Jan. 31, 2021, 10:46 p.m. UTC
The simple pipeline handler has grown over time, and isn't that simple
anymore that it can easily be understood by an unfamiliar reader.
Document the design to explicitly state the expectations of the pipeline
handler, and to explain how it operates.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 83 ++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

Comments

Kieran Bingham March 1, 2021, 8:23 p.m. UTC | #1
On 31/01/2021 22:46, Laurent Pinchart wrote:
> The simple pipeline handler has grown over time, and isn't that simple
> anymore that it can easily be understood by an unfamiliar reader.
> Document the design to explicitly state the expectations of the pipeline
> handler, and to explain how it operates.

Should it be renamed?

GenericPipelineHandler... something else ?


> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 83 ++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index f072e0f1fa81..7c5a56a2f395 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -38,6 +38,89 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(SimplePipeline)
>  
> +/* -----------------------------------------------------------------------------
> + *
> + * Overview
> + * --------
> + *
> + * The SimplePipelineHandler relies on generic kernel APIs to control a camera
> + * device, without any device-specific code and with limited device-specific
> + * static data.
> + *
> + * To quality for support by the simple pipeline handler, a device shall

s/quality/qualify/

> + *
> + * - be supported by V4L2 drivers, exposing the Media Controller API, the V4L2
> + *   subdev APIs and the media bus format-based enumeration extension for the
> + *   VIDIOC_ENUM_FMT ioctl ;
> + * - not expose any device-specific API from drivers to userspace ;
> + * - include one or more camera sensor media entities and one or more video
> + *   capture devices ;
> + * - have a capture pipeline with linear paths from the camera sensors to the
> + *   video capture devices ; and
> + * - have an optional memory-to-memory device to perform format conversion
> + *   and/or scaling, exposed as a V4L2 M2M device.
> + *
> + * As devices that require a specific pipeline handler may still match the
> + * above characteristics, the simple pipeline handler doesn't attempt to
> + * automatically determine which devices it can support. It instead relies on
> + * an explicit list of supported devices, provided in the supportedDevices
> + * array.
> + *
> + * When matching a device, the pipeline handler enumerates all camera sensors
> + * and attempts, for each of them, to find a path to a video captude video node.

s/captude/capture/

> + * It does so by traversing the media graph, following the first non permanently
> + * disabled downstream link. If such a path is found, the pipeline handler
> + * creates a corresponding SimpleCameraData instance, and stores the media graph
> + * path in its entities_ list.
> + *
> + * A more complex graph search algorithm could be implemented if a device that
> + * would otherwise be compatible with the pipeline handler isn't correctly
> + * handled by this heuristic.
> + *
> + * Once the camera data instances have been created, the match() function
> + * creates a V4L2Subdevice instance for each entity used by any of the cameras
> + * and stores the instances in SimplePipelineHandler::subdevs_, accessible by
> + * the SimpleCameraData class through the SimplePipelineHandler::subdev()
> + * function. This avoids duplication of subdev instances between different
> + * cameras when the same entity is used in multiple paths. A similar mechanism
> + * is used for V4L2VideoDevice instances, but instances are in this case created
> + * on demand when access through SimplePipelineHandler::video() instead of all

s/access/accessed/

> + * in one go at initialization time.
> + *
> + * Finally, all camera data instances are initialized to gather information
> + * about the possible pipeline configurations for the corresponding camera. If
> + * valid pipeline configurations are found, a Camera is registered for the
> + * SimpleCameraData instance.
> + *
> + * Pipeline Configuration
> + * ----------------------
> + *
> + * The simple pipeline handler configures the pipeline by propagating V4L2
> + * subdev formats from the camera sensor to the video node. The format is first
> + * set on the camera sensor's output, using the native camera sensor
> + * resolution. Then, on every link in the pipeline, the format is retrieved on
> + * the link source and set unmodified on the link sink.
> + *
> + * When initializating the camera data, this above procedure is repeated for
> + * every media bus format supported by the camera sensor. Upon reaching the
> + * video node, the pixel formats compatible with the media bus format are
> + * enumerated. Each of those pixel formats correspond to one possible pipeline

s/correspond/corresponds/	

> + * configuration, stored as an intsance of SimpleCameraData::Configuration in

s/intsance/instance/

> + * the SimpleCameraData::formats_ map.
> + *
> + * Format Conversion and Scaling
> + * -----------------------------
> + *
> + * The capture pipeline isn't expected to include a scaler, and if a scaler is
> + * available, it is ignored when configuring the pipeline. However, the simple
> + * pipeline handler supports optional memory-to-memory converters to scale the
> + * image and convert it to a different pixel format. If such a converter is
> + * present, the pipeline handler enumerates, for each pipeline configuration,
> + * the pixel formats and sizes that the converter can produce for the output of
> + * the capture video node, and stores the information in the outputFormats and
> + * outputSizes of the SimpleCameraData::Configuration structure.
> + */
> +
>  class SimplePipelineHandler;
>  

With all that fixed,

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  struct SimplePipelineInfo {
>
Laurent Pinchart March 1, 2021, 10:46 p.m. UTC | #2
Hi Kieran,

On Mon, Mar 01, 2021 at 08:23:54PM +0000, Kieran Bingham wrote:
> On 31/01/2021 22:46, Laurent Pinchart wrote:
> > The simple pipeline handler has grown over time, and isn't that simple
> > anymore that it can easily be understood by an unfamiliar reader.
> > Document the design to explicitly state the expectations of the pipeline
> > handler, and to explain how it operates.
> 
> Should it be renamed?
> 
> GenericPipelineHandler... something else ?

Possibly, especially that it will get less and less simple over time :-)
I'd rather do this on top of this series though.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 83 ++++++++++++++++++++++++
> >  1 file changed, 83 insertions(+)
> > 
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index f072e0f1fa81..7c5a56a2f395 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -38,6 +38,89 @@ namespace libcamera {
> >  
> >  LOG_DEFINE_CATEGORY(SimplePipeline)
> >  
> > +/* -----------------------------------------------------------------------------
> > + *
> > + * Overview
> > + * --------
> > + *
> > + * The SimplePipelineHandler relies on generic kernel APIs to control a camera
> > + * device, without any device-specific code and with limited device-specific
> > + * static data.
> > + *
> > + * To quality for support by the simple pipeline handler, a device shall
> 
> s/quality/qualify/
> 
> > + *
> > + * - be supported by V4L2 drivers, exposing the Media Controller API, the V4L2
> > + *   subdev APIs and the media bus format-based enumeration extension for the
> > + *   VIDIOC_ENUM_FMT ioctl ;
> > + * - not expose any device-specific API from drivers to userspace ;
> > + * - include one or more camera sensor media entities and one or more video
> > + *   capture devices ;
> > + * - have a capture pipeline with linear paths from the camera sensors to the
> > + *   video capture devices ; and
> > + * - have an optional memory-to-memory device to perform format conversion
> > + *   and/or scaling, exposed as a V4L2 M2M device.
> > + *
> > + * As devices that require a specific pipeline handler may still match the
> > + * above characteristics, the simple pipeline handler doesn't attempt to
> > + * automatically determine which devices it can support. It instead relies on
> > + * an explicit list of supported devices, provided in the supportedDevices
> > + * array.
> > + *
> > + * When matching a device, the pipeline handler enumerates all camera sensors
> > + * and attempts, for each of them, to find a path to a video captude video node.
> 
> s/captude/capture/
> 
> > + * It does so by traversing the media graph, following the first non permanently
> > + * disabled downstream link. If such a path is found, the pipeline handler
> > + * creates a corresponding SimpleCameraData instance, and stores the media graph
> > + * path in its entities_ list.
> > + *
> > + * A more complex graph search algorithm could be implemented if a device that
> > + * would otherwise be compatible with the pipeline handler isn't correctly
> > + * handled by this heuristic.
> > + *
> > + * Once the camera data instances have been created, the match() function
> > + * creates a V4L2Subdevice instance for each entity used by any of the cameras
> > + * and stores the instances in SimplePipelineHandler::subdevs_, accessible by
> > + * the SimpleCameraData class through the SimplePipelineHandler::subdev()
> > + * function. This avoids duplication of subdev instances between different
> > + * cameras when the same entity is used in multiple paths. A similar mechanism
> > + * is used for V4L2VideoDevice instances, but instances are in this case created
> > + * on demand when access through SimplePipelineHandler::video() instead of all
> 
> s/access/accessed/
> 
> > + * in one go at initialization time.
> > + *
> > + * Finally, all camera data instances are initialized to gather information
> > + * about the possible pipeline configurations for the corresponding camera. If
> > + * valid pipeline configurations are found, a Camera is registered for the
> > + * SimpleCameraData instance.
> > + *
> > + * Pipeline Configuration
> > + * ----------------------
> > + *
> > + * The simple pipeline handler configures the pipeline by propagating V4L2
> > + * subdev formats from the camera sensor to the video node. The format is first
> > + * set on the camera sensor's output, using the native camera sensor
> > + * resolution. Then, on every link in the pipeline, the format is retrieved on
> > + * the link source and set unmodified on the link sink.
> > + *
> > + * When initializating the camera data, this above procedure is repeated for
> > + * every media bus format supported by the camera sensor. Upon reaching the
> > + * video node, the pixel formats compatible with the media bus format are
> > + * enumerated. Each of those pixel formats correspond to one possible pipeline
> 
> s/correspond/corresponds/	
> 
> > + * configuration, stored as an intsance of SimpleCameraData::Configuration in
> 
> s/intsance/instance/
> 
> > + * the SimpleCameraData::formats_ map.
> > + *
> > + * Format Conversion and Scaling
> > + * -----------------------------
> > + *
> > + * The capture pipeline isn't expected to include a scaler, and if a scaler is
> > + * available, it is ignored when configuring the pipeline. However, the simple
> > + * pipeline handler supports optional memory-to-memory converters to scale the
> > + * image and convert it to a different pixel format. If such a converter is
> > + * present, the pipeline handler enumerates, for each pipeline configuration,
> > + * the pixel formats and sizes that the converter can produce for the output of
> > + * the capture video node, and stores the information in the outputFormats and
> > + * outputSizes of the SimpleCameraData::Configuration structure.
> > + */
> > +
> >  class SimplePipelineHandler;
> >  
> 
> With all that fixed,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> >  struct SimplePipelineInfo {
> >
Paul Elder March 2, 2021, 12:55 a.m. UTC | #3
Hi Laurent,

On Mon, Feb 01, 2021 at 12:46:54AM +0200, Laurent Pinchart wrote:
> The simple pipeline handler has grown over time, and isn't that simple
> anymore that it can easily be understood by an unfamiliar reader.
> Document the design to explicitly state the expectations of the pipeline
> handler, and to explain how it operates.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

With all the fixes suggested by Kieran,

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 83 ++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index f072e0f1fa81..7c5a56a2f395 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -38,6 +38,89 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(SimplePipeline)
>  
> +/* -----------------------------------------------------------------------------
> + *
> + * Overview
> + * --------
> + *
> + * The SimplePipelineHandler relies on generic kernel APIs to control a camera
> + * device, without any device-specific code and with limited device-specific
> + * static data.
> + *
> + * To quality for support by the simple pipeline handler, a device shall
> + *
> + * - be supported by V4L2 drivers, exposing the Media Controller API, the V4L2
> + *   subdev APIs and the media bus format-based enumeration extension for the
> + *   VIDIOC_ENUM_FMT ioctl ;
> + * - not expose any device-specific API from drivers to userspace ;
> + * - include one or more camera sensor media entities and one or more video
> + *   capture devices ;
> + * - have a capture pipeline with linear paths from the camera sensors to the
> + *   video capture devices ; and
> + * - have an optional memory-to-memory device to perform format conversion
> + *   and/or scaling, exposed as a V4L2 M2M device.
> + *
> + * As devices that require a specific pipeline handler may still match the
> + * above characteristics, the simple pipeline handler doesn't attempt to
> + * automatically determine which devices it can support. It instead relies on
> + * an explicit list of supported devices, provided in the supportedDevices
> + * array.
> + *
> + * When matching a device, the pipeline handler enumerates all camera sensors
> + * and attempts, for each of them, to find a path to a video captude video node.
> + * It does so by traversing the media graph, following the first non permanently
> + * disabled downstream link. If such a path is found, the pipeline handler
> + * creates a corresponding SimpleCameraData instance, and stores the media graph
> + * path in its entities_ list.
> + *
> + * A more complex graph search algorithm could be implemented if a device that
> + * would otherwise be compatible with the pipeline handler isn't correctly
> + * handled by this heuristic.
> + *
> + * Once the camera data instances have been created, the match() function
> + * creates a V4L2Subdevice instance for each entity used by any of the cameras
> + * and stores the instances in SimplePipelineHandler::subdevs_, accessible by
> + * the SimpleCameraData class through the SimplePipelineHandler::subdev()
> + * function. This avoids duplication of subdev instances between different
> + * cameras when the same entity is used in multiple paths. A similar mechanism
> + * is used for V4L2VideoDevice instances, but instances are in this case created
> + * on demand when access through SimplePipelineHandler::video() instead of all
> + * in one go at initialization time.
> + *
> + * Finally, all camera data instances are initialized to gather information
> + * about the possible pipeline configurations for the corresponding camera. If
> + * valid pipeline configurations are found, a Camera is registered for the
> + * SimpleCameraData instance.
> + *
> + * Pipeline Configuration
> + * ----------------------
> + *
> + * The simple pipeline handler configures the pipeline by propagating V4L2
> + * subdev formats from the camera sensor to the video node. The format is first
> + * set on the camera sensor's output, using the native camera sensor
> + * resolution. Then, on every link in the pipeline, the format is retrieved on
> + * the link source and set unmodified on the link sink.
> + *
> + * When initializating the camera data, this above procedure is repeated for
> + * every media bus format supported by the camera sensor. Upon reaching the
> + * video node, the pixel formats compatible with the media bus format are
> + * enumerated. Each of those pixel formats correspond to one possible pipeline
> + * configuration, stored as an intsance of SimpleCameraData::Configuration in
> + * the SimpleCameraData::formats_ map.
> + *
> + * Format Conversion and Scaling
> + * -----------------------------
> + *
> + * The capture pipeline isn't expected to include a scaler, and if a scaler is
> + * available, it is ignored when configuring the pipeline. However, the simple
> + * pipeline handler supports optional memory-to-memory converters to scale the
> + * image and convert it to a different pixel format. If such a converter is
> + * present, the pipeline handler enumerates, for each pipeline configuration,
> + * the pixel formats and sizes that the converter can produce for the output of
> + * the capture video node, and stores the information in the outputFormats and
> + * outputSizes of the SimpleCameraData::Configuration structure.
> + */
> +
>  class SimplePipelineHandler;
>  
>  struct SimplePipelineInfo {
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index f072e0f1fa81..7c5a56a2f395 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -38,6 +38,89 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(SimplePipeline)
 
+/* -----------------------------------------------------------------------------
+ *
+ * Overview
+ * --------
+ *
+ * The SimplePipelineHandler relies on generic kernel APIs to control a camera
+ * device, without any device-specific code and with limited device-specific
+ * static data.
+ *
+ * To quality for support by the simple pipeline handler, a device shall
+ *
+ * - be supported by V4L2 drivers, exposing the Media Controller API, the V4L2
+ *   subdev APIs and the media bus format-based enumeration extension for the
+ *   VIDIOC_ENUM_FMT ioctl ;
+ * - not expose any device-specific API from drivers to userspace ;
+ * - include one or more camera sensor media entities and one or more video
+ *   capture devices ;
+ * - have a capture pipeline with linear paths from the camera sensors to the
+ *   video capture devices ; and
+ * - have an optional memory-to-memory device to perform format conversion
+ *   and/or scaling, exposed as a V4L2 M2M device.
+ *
+ * As devices that require a specific pipeline handler may still match the
+ * above characteristics, the simple pipeline handler doesn't attempt to
+ * automatically determine which devices it can support. It instead relies on
+ * an explicit list of supported devices, provided in the supportedDevices
+ * array.
+ *
+ * When matching a device, the pipeline handler enumerates all camera sensors
+ * and attempts, for each of them, to find a path to a video captude video node.
+ * It does so by traversing the media graph, following the first non permanently
+ * disabled downstream link. If such a path is found, the pipeline handler
+ * creates a corresponding SimpleCameraData instance, and stores the media graph
+ * path in its entities_ list.
+ *
+ * A more complex graph search algorithm could be implemented if a device that
+ * would otherwise be compatible with the pipeline handler isn't correctly
+ * handled by this heuristic.
+ *
+ * Once the camera data instances have been created, the match() function
+ * creates a V4L2Subdevice instance for each entity used by any of the cameras
+ * and stores the instances in SimplePipelineHandler::subdevs_, accessible by
+ * the SimpleCameraData class through the SimplePipelineHandler::subdev()
+ * function. This avoids duplication of subdev instances between different
+ * cameras when the same entity is used in multiple paths. A similar mechanism
+ * is used for V4L2VideoDevice instances, but instances are in this case created
+ * on demand when access through SimplePipelineHandler::video() instead of all
+ * in one go at initialization time.
+ *
+ * Finally, all camera data instances are initialized to gather information
+ * about the possible pipeline configurations for the corresponding camera. If
+ * valid pipeline configurations are found, a Camera is registered for the
+ * SimpleCameraData instance.
+ *
+ * Pipeline Configuration
+ * ----------------------
+ *
+ * The simple pipeline handler configures the pipeline by propagating V4L2
+ * subdev formats from the camera sensor to the video node. The format is first
+ * set on the camera sensor's output, using the native camera sensor
+ * resolution. Then, on every link in the pipeline, the format is retrieved on
+ * the link source and set unmodified on the link sink.
+ *
+ * When initializating the camera data, this above procedure is repeated for
+ * every media bus format supported by the camera sensor. Upon reaching the
+ * video node, the pixel formats compatible with the media bus format are
+ * enumerated. Each of those pixel formats correspond to one possible pipeline
+ * configuration, stored as an intsance of SimpleCameraData::Configuration in
+ * the SimpleCameraData::formats_ map.
+ *
+ * Format Conversion and Scaling
+ * -----------------------------
+ *
+ * The capture pipeline isn't expected to include a scaler, and if a scaler is
+ * available, it is ignored when configuring the pipeline. However, the simple
+ * pipeline handler supports optional memory-to-memory converters to scale the
+ * image and convert it to a different pixel format. If such a converter is
+ * present, the pipeline handler enumerates, for each pipeline configuration,
+ * the pixel formats and sizes that the converter can produce for the output of
+ * the capture video node, and stores the information in the outputFormats and
+ * outputSizes of the SimpleCameraData::Configuration structure.
+ */
+
 class SimplePipelineHandler;
 
 struct SimplePipelineInfo {