| Message ID | 20220428105219.4b068b1f.dorota.czaplejewicz@puri.sm (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCHv2,1/2] doc/media api: Try to make enum usage clearer | expand |
Hi Dorota On Thu, Apr 28, 2022 at 10:52:19AM +0200, Dorota Czaplejewicz wrote: > Fixed: typo "format" -> "frame size" in enum-frame-size > Added: no holes in the enumeration > Added: enumerations per what? > Added: who fills in what in calls > Changed: "zero" -> "0" > Changed: "given" -> "specified" Empty line here > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> > --- > .../v4l/vidioc-subdev-enum-frame-size.rst | 44 ++++++++++++------- > 1 file changed, 28 insertions(+), 16 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..2c6fd291dc44 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 enumeration of frame sizes supported by over 80 cols > +a sub-device on the specified pad for the specified media bus format. > +Supported formats can be retrieved with the This seems quite an arbitrary change. What's wrong with the existing phrase ? > :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 enumerations 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 enumeration. > +Each enumeeration starts with the ``index`` of 0, and s/enumeeration/enumeration/ > +the lowest invalid index marks the end of the enumeration. > + > +Therefore, to enumerate frame sizes allowed on the specified pad > +and using the specified 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 enumeration, > +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 Rougue line break > + belonging to the given pad and format. Filled in by the application. > * - __u32 > - ``pad`` > - - Pad number as reported by the media controller API. > + - Pad number as reported by the media controller API. Filled in by the application. over 80 cols > * - __u32 > - ``code`` > - The media bus format code, as defined in > - :ref:`v4l2-mbus-format`. > + :ref:`v4l2-mbus-format`. Filled in by the application. > * - __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 Even more than 1/2, I am a bit failing to see what is missing in the existing doc. If it feels better to others who will have a look, I for sure won't oppose this change though :) > -- > 2.35.1 >
Hi, On Thu, 28 Apr 2022 15:11:46 +0200 Jacopo Mondi <jacopo@jmondi.org> wrote: > Hi Dorota > > On Thu, Apr 28, 2022 at 10:52:19AM +0200, Dorota Czaplejewicz wrote: > > Fixed: typo "format" -> "frame size" in enum-frame-size > > Added: no holes in the enumeration > > Added: enumerations per what? > > Added: who fills in what in calls > > Changed: "zero" -> "0" > > Changed: "given" -> "specified" > > Empty line here > > > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> > > --- > > .../v4l/vidioc-subdev-enum-frame-size.rst | 44 ++++++++++++------- > > 1 file changed, 28 insertions(+), 16 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..2c6fd291dc44 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 enumeration of frame sizes supported by > > over 80 cols > > > +a sub-device on the specified pad for the specified media bus format. > > +Supported formats can be retrieved with the > > This seems quite an arbitrary change. What's wrong with the existing > phrase ? > "Given" is vague and does not really say who gives and who is given. Is it the kernel or the application? It kept confusing me. I tried to turn it into a "selected", which has more "application" vibes, but I was asked to change it to "specified". IMO this could benefit from an active voice, but I didn't want to rewrite it completely. About "access the enumeration" - this serves to show the data structure up front. In my original patch it was "array", which hopefully implies some things: that it's continuous and indexed. I was asked to get rid of "array" though. Thanks, Dorota > > :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 enumerations 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 enumeration. > > +Each enumeeration starts with the ``index`` of 0, and > > s/enumeeration/enumeration/ > > > +the lowest invalid index marks the end of the enumeration. > > + > > +Therefore, to enumerate frame sizes allowed on the specified pad > > +and using the specified 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 enumeration, > > +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 > > Rougue line break > > > + belonging to the given pad and format. Filled in by the application. > > * - __u32 > > - ``pad`` > > - - Pad number as reported by the media controller API. > > + - Pad number as reported by the media controller API. Filled in by the application. > > over 80 cols > > > * - __u32 > > - ``code`` > > - The media bus format code, as defined in > > - :ref:`v4l2-mbus-format`. > > + :ref:`v4l2-mbus-format`. Filled in by the application. > > * - __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 > > Even more than 1/2, I am a bit failing to see what is missing in the > existing doc. If it feels better to others who will have a look, I for sure > won't oppose this change though :) > > > -- > > 2.35.1 > > > >
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..2c6fd291dc44 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 enumeration of frame sizes supported by +a sub-device on the specified pad for the specified 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 enumerations 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 enumeration. +Each enumeeration starts with the ``index`` of 0, and +the lowest invalid index marks the end of the enumeration. + +Therefore, to enumerate frame sizes allowed on the specified pad +and using the specified 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 enumeration, +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 application. * - __u32 - ``pad`` - - Pad number as reported by the media controller API. + - Pad number as reported by the media controller API. Filled in by the application. * - __u32 - ``code`` - The media bus format code, as defined in - :ref:`v4l2-mbus-format`. + :ref:`v4l2-mbus-format`. Filled in by the application. * - __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
Fixed: typo "format" -> "frame size" in enum-frame-size Added: no holes in the enumeration Added: enumerations per what? Added: who fills in what in calls Changed: "zero" -> "0" Changed: "given" -> "specified" Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> --- .../v4l/vidioc-subdev-enum-frame-size.rst | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-)