* [PATCHv2 2/2] media api: Try to make enum usage clearer
[not found] <20220428083715.75997-1-dorota.czaplejewicz@puri.sm>
@ 2022-04-28 8:52 ` Dorota Czaplejewicz
2022-04-28 13:11 ` Jacopo Mondi
0 siblings, 1 reply; 3+ messages in thread
From: Dorota Czaplejewicz @ 2022-04-28 8:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, linux-media, linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 4060 bytes --]
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(-)
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
--
2.35.1
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCHv2 2/2] media api: Try to make enum usage clearer
2022-04-28 8:52 ` [PATCHv2 2/2] media api: Try to make enum usage clearer Dorota Czaplejewicz
@ 2022-04-28 13:11 ` Jacopo Mondi
2022-04-28 14:04 ` Dorota Czaplejewicz
0 siblings, 1 reply; 3+ messages in thread
From: Jacopo Mondi @ 2022-04-28 13:11 UTC (permalink / raw)
To: Dorota Czaplejewicz
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 4583 bytes --]
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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCHv2 2/2] media api: Try to make enum usage clearer
2022-04-28 13:11 ` Jacopo Mondi
@ 2022-04-28 14:04 ` Dorota Czaplejewicz
0 siblings, 0 replies; 3+ messages in thread
From: Dorota Czaplejewicz @ 2022-04-28 14:04 UTC (permalink / raw)
To: Jacopo Mondi; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 5648 bytes --]
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
> >
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-28 14:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220428083715.75997-1-dorota.czaplejewicz@puri.sm>
2022-04-28 8:52 ` [PATCHv2 2/2] media api: Try to make enum usage clearer Dorota Czaplejewicz
2022-04-28 13:11 ` Jacopo Mondi
2022-04-28 14:04 ` Dorota Czaplejewicz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).