Message-ID: <20220422092542.1333177-1-dorota.czaplejewicz@puri.sm>
Date:   Fri, 22 Apr 2022 11:30:54 +0200
From:   Dorota Czaplejewicz <dorota.czaplejewicz@...i.sm>
To:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel@...i.sm
Subject: [PATCH] Documentation/media: Try to make enum usage clearer

Fixed: typo "format" -> "frame size" in enum-frame-size
Added: mbus codes must not repeat
Added: no holes in the array
Added: arrays per what?
Added: who fills in what (questionable naming: caller/driver. caller/callee could be confusing for beginners, they look similar)
Changed: "zero" -> "0"
Changed: "given" -> "selected". "given to me by the driver?" dunno feels like "selected" or "chosen" are the words to indicate agency to the consumer

---
Hello,

I encountered some difficulties trying to implement a media-api driver, and I traced the problems to not understanding the documentation. Here I try to make the API usage clearer, after consulting with libcamera authors.

Still unclear how it works so I didn't touch it: "which". What is a "try format" vs "active format"?

Cheers,
Dorota Czaplejewicz


 .../v4l/vidioc-subdev-enum-frame-size.rst     | 44 ++++++++++++-------
 .../v4l/vidioc-subdev-enum-mbus-code.rst      | 39 +++++++++++-----
 2 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-frame-size.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-frame-size.rst
index c25a9896df0e..c7afeffb6269 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-frame-size.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-frame-size.rst
@@ -31,18 +31,29 @@ Arguments
 Description
 ===========
 
-This ioctl allows applications to enumerate all frame sizes supported by
-a sub-device on the given pad for the given media bus format. Supported
-formats can be retrieved with the
+This ioctl allows applications to access the array of frame sizes supported by
+a sub-device on the selected pad for the selected media bus format.
+Supported formats can be retrieved with the
 :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE`
 ioctl.
 
-To enumerate frame sizes applications initialize the ``pad``, ``which``
-, ``code`` and ``index`` fields of the struct
-:c:type:`v4l2_subdev_mbus_code_enum` and
-call the :ref:`VIDIOC_SUBDEV_ENUM_FRAME_SIZE` ioctl with a pointer to the
-structure. Drivers fill the minimum and maximum frame sizes or return an
-EINVAL error code if one of the input parameters is invalid.
+The arrays are defined by the driver, and indexed using the ``index`` field
+of the struct :c:type:`v4l2_subdev_mbus_code_enum`.
+Each pair of ``pad`` and ``code`` correspond to a separate array.
+Each array starts with the ``index`` of 0, and
+the first invalid index marks the end of array.
+
+Therefore, to enumerate frame sizes allowed on the selected pad
+and using the selected mbus format, initialize the
+``pad``, ``which``, and ``code`` fields to desired values,
+and set ``index`` to 0.
+Then call the :ref:`VIDIOC_SUBDEV_ENUM_FRAME_SIZE` ioctl with a pointer to the
+structure.
+
+A successful call will return with minimum and maximum frame sizes filled in.
+Repeat with increasing ``index`` until ``EINVAL`` is received.
+``EINVAL`` means that either no more entries are available in the array,
+or that an input parameter was invalid.
 
 Sub-devices that only support discrete frame sizes (such as most
 sensors) will return one or more frame sizes with identical minimum and
@@ -72,26 +83,27 @@ information about try formats.
 
     * - __u32
       - ``index``
-      - Number of the format in the enumeration, set by the application.
+      - Index of the frame size in the enumeration
+    belonging to the given pad and format. Filled in by the caller.
     * - __u32
       - ``pad``
-      - Pad number as reported by the media controller API.
+      - Pad number as reported by the media controller API. Filled in by the caller.
     * - __u32
       - ``code``
       - The media bus format code, as defined in
-	:ref:`v4l2-mbus-format`.
+	:ref:`v4l2-mbus-format`. Filled in by the caller.
     * - __u32
       - ``min_width``
-      - Minimum frame width, in pixels.
+      - Minimum frame width, in pixels. Filled in by the driver.
     * - __u32
       - ``max_width``
-      - Maximum frame width, in pixels.
+      - Maximum frame width, in pixels. Filled in by the driver.
     * - __u32
       - ``min_height``
-      - Minimum frame height, in pixels.
+      - Minimum frame height, in pixels. Filled in by the driver.
     * - __u32
       - ``max_height``
-      - Maximum frame height, in pixels.
+      - Maximum frame height, in pixels. Filled in by the driver.
     * - __u32
       - ``which``
       - Frame sizes to be enumerated, from enum
diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
index 417f1a19bcc4..22d046dd09c2 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
@@ -31,15 +31,29 @@ Arguments
 Description
 ===========
 
-To enumerate media bus formats available at a given sub-device pad
-applications initialize the ``pad``, ``which`` and ``index`` fields of
-struct
-:c:type:`v4l2_subdev_mbus_code_enum` and
-call the :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE` ioctl with a pointer to this
-structure. Drivers fill the rest of the structure or return an ``EINVAL``
-error code if either the ``pad`` or ``index`` are invalid. All media bus
-formats are enumerable by beginning at index zero and incrementing by
-one until ``EINVAL`` is returned.
+This call is used by the application to access the array of bus formats
+for the selected pad.
+
+The arrays are defined by the driver, and indexed using the ``index`` field
+of struct :c:type:`v4l2_subdev_mbus_code_enum`.
+Each value of ``pad`` corresponds to a separate array.
+Each array starts with the ``index`` of 0, and
+the first invalid index marks the end of array.
+
+Therefore, to enumerate media bus formats available at a given sub-device pad,
+initialize the ``pad``, and ``which`` fields to desired values,
+and set ``index`` to 0.
+Then call the :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE` ioctl
+with a pointer to this structure.
+
+A successful call will return with the ``code`` field filled in
+with a mbus format value.
+Repeat with increasing ``index`` until ``EINVAL`` is received.
+``EINVAL`` means that either ``pad`` is invalid,
+or that there are no more codes available at this pad.
+
+The driver must not return the same value of ``code`` for different indices
+at the same pad.
 
 Available media bus formats may depend on the current 'try' formats at
 other pads of the sub-device, as well as on the current active links.
@@ -57,14 +71,15 @@ information about the try formats.
 
     * - __u32
       - ``pad``
-      - Pad number as reported by the media controller API.
+      - Pad number as reported by the media controller API. Filled in by the caller.
     * - __u32
       - ``index``
-      - Number of the format in the enumeration, set by the application.
+      - Index of the mbus code in the enumeration belonging to the given pad.
+    Filled in by the caller.
     * - __u32
       - ``code``
       - The media bus format code, as defined in
-	:ref:`v4l2-mbus-format`.
+	:ref:`v4l2-mbus-format`. Filled in by the driver.
     * - __u32
       - ``which``
       - Media bus format codes to be enumerated, from enum
-- 
2.34.1


Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists