Re: [PATCH v2.2] v4l: Clearly document interactions between formats, controls and buffers
Hi Hans, On Monday 06 Mar 2017 11:46:28 Hans Verkuil wrote: > On 06/03/17 11:35, Laurent Pinchart wrote: > > On Monday 06 Mar 2017 11:04:50 Hans Verkuil wrote: > >> On 05/03/17 22:36, Laurent Pinchart wrote: > >>> V4L2 exposes parameters that influence buffers sizes through the format > >>> ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT, VIDIOC_S_FMT, and possibly > >>> VIDIOC_G_SELECTION and VIDIOC_S_SELECTION). Other parameters not part of > >>> the format structure may also influence buffer sizes or buffer layout in > >>> general. One existing such parameter is rotation, which is implemented > >>> by the V4L2_CID_ROTATE control and thus exposed through the V4L2 control > >>> ioctls. > >>> > >>> The interaction between those parameters and buffers is currently only > >>> partially specified by the V4L2 API. In particular interactions between > >>> controls and buffers isn't specified at all. The behaviour of the > >>> VIDIOC_S_FMT and VIDIOC_S_SELECTION ioctls when buffers are allocated is > >>> also not fully specified. > >>> > >>> This patch clearly defines and documents the interactions between > >>> formats, selections, controls and buffers. > >>> > >>> The preparatory discussions for the documentation change considered > >>> completely disallowing controls that change the buffer size or layout, > >>> in favour of extending the format API with a new ioctl that would bundle > >>> those controls with format information. The idea has been rejected, as > >>> this would essentially be a restricted version of the upcoming request > >>> API that wouldn't bring any additional value. > >>> > >>> Another option we have considered was to mandate the use of the request > >>> API to modify controls that influence buffer size or layout. This has > >>> also been rejected on the grounds that requiring the request API to > >>> change rotation even when streaming is stopped would significantly > >>> complicate implementation of drivers and usage of the V4L2 API for > >>> applications. > >>> > >>> Applications will however be required to use the upcoming request API to > >>> change at runtime formats or controls that influence the buffer size or > >>> layout, because of the need to synchronize buffers with the formats and > >>> controls. Otherwise there would be no way to interpret the content of a > >>> buffer correctly. > >>> > >>> Signed-off-by: Laurent Pinchart > >>> > >>> Acked-by: Sakari Ailus > >>> --- > >>> Changes since v2.1: > >>> > >>> - Fixed small issues in commit message > >>> - Simplified wording of one sentence in the documentation > >>> > >>> Changes since v2: > >>> > >>> - Document the interaction with ioctls that can affect formats > >>> (VIDIOC_S_SELECTION, VIDIOC_S_INPUT, VIDIOC_S_OUTPUT, VIDIOC_S_STD and > >>> VIDIOC_S_DV_TIMINGS) > >>> - Clarify the format/control change order > >>> --- > >>> > >>> Documentation/media/uapi/v4l/buffer.rst | 108 > >>> 1 file changed, 108 insertions(+) > >>> > >>> diff --git a/Documentation/media/uapi/v4l/buffer.rst > >>> b/Documentation/media/uapi/v4l/buffer.rst index > >>> ac58966ccb9b..60d62a5824f8 100644 > >>> --- a/Documentation/media/uapi/v4l/buffer.rst > >>> +++ b/Documentation/media/uapi/v4l/buffer.rst > >>> @@ -34,6 +34,114 @@ flags are copied from the OUTPUT video buffer to the > >>> CAPTURE video> > >>> > >>> buffer. > >>> > >>> +Interactions between formats, controls and buffers > >>> +== > >>> + > >>> +V4L2 exposes parameters that influence the buffer size, or the way data > >>> is +laid out in the buffer. Those parameters are exposed through both > >>> formats and +controls. One example of such a control is the > >>> ``V4L2_CID_ROTATE`` control +that modifies the direction in which pixels > >>> are stored in the buffer, as well +as the buffer size when the selected > >>> format includes padding at the end of +lines. > >>> + > >>> +The set of information needed to interpret the content of a buffer > >>> (e.g. the +pixel format, the line stride, the tiling orientation or the > >>> rotation) is +collectively referred to in the rest of this section as > >>> the buffer layout. > >>> + > >>> +Modifying formats or controls that influence the buffer size or layout > >>> require +the stream to be stopped. Any attempt at such a modification > >>> while the stream +is active shall cause the ioctl setting the format or > >>> the control to return +the ``EBUSY`` error code. > >> > >> This is my problem with putting the more complex case first: if you are > >> reading this for the first time then the preceding paragraph is simply > >> *wrong*. > >> > >> You cannot modify the buffer size when the stream is stopped. You need to > >> free all buffers first before you can do that. > > > > That's driver-dependent. If I reverse the order to start with the more > > restricted case, it will also be wrong for drivers that implement the > > other case. > > It's not about wrong or right,
Re: [PATCH v2.2] v4l: Clearly document interactions between formats, controls and buffers
On 06/03/17 11:35, Laurent Pinchart wrote: Hi Hans, On Monday 06 Mar 2017 11:04:50 Hans Verkuil wrote: On 05/03/17 22:36, Laurent Pinchart wrote: V4L2 exposes parameters that influence buffers sizes through the format ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT, VIDIOC_S_FMT, and possibly VIDIOC_G_SELECTION and VIDIOC_S_SELECTION). Other parameters not part of the format structure may also influence buffer sizes or buffer layout in general. One existing such parameter is rotation, which is implemented by the V4L2_CID_ROTATE control and thus exposed through the V4L2 control ioctls. The interaction between those parameters and buffers is currently only partially specified by the V4L2 API. In particular interactions between controls and buffers isn't specified at all. The behaviour of the VIDIOC_S_FMT and VIDIOC_S_SELECTION ioctls when buffers are allocated is also not fully specified. This patch clearly defines and documents the interactions between formats, selections, controls and buffers. The preparatory discussions for the documentation change considered completely disallowing controls that change the buffer size or layout, in favour of extending the format API with a new ioctl that would bundle those controls with format information. The idea has been rejected, as this would essentially be a restricted version of the upcoming request API that wouldn't bring any additional value. Another option we have considered was to mandate the use of the request API to modify controls that influence buffer size or layout. This has also been rejected on the grounds that requiring the request API to change rotation even when streaming is stopped would significantly complicate implementation of drivers and usage of the V4L2 API for applications. Applications will however be required to use the upcoming request API to change at runtime formats or controls that influence the buffer size or layout, because of the need to synchronize buffers with the formats and controls. Otherwise there would be no way to interpret the content of a buffer correctly. Signed-off-by: Laurent Pinchart Acked-by: Sakari Ailus --- Changes since v2.1: - Fixed small issues in commit message - Simplified wording of one sentence in the documentation Changes since v2: - Document the interaction with ioctls that can affect formats (VIDIOC_S_SELECTION, VIDIOC_S_INPUT, VIDIOC_S_OUTPUT, VIDIOC_S_STD and VIDIOC_S_DV_TIMINGS) - Clarify the format/control change order --- Documentation/media/uapi/v4l/buffer.rst | 108 +++ 1 file changed, 108 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ac58966ccb9b..60d62a5824f8 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -34,6 +34,114 @@ flags are copied from the OUTPUT video buffer to the CAPTURE video> buffer. +Interactions between formats, controls and buffers +== + +V4L2 exposes parameters that influence the buffer size, or the way data is +laid out in the buffer. Those parameters are exposed through both formats and +controls. One example of such a control is the ``V4L2_CID_ROTATE`` control +that modifies the direction in which pixels are stored in the buffer, as well +as the buffer size when the selected format includes padding at the end of +lines. + +The set of information needed to interpret the content of a buffer (e.g. the +pixel format, the line stride, the tiling orientation or the rotation) is +collectively referred to in the rest of this section as the buffer layout. + +Modifying formats or controls that influence the buffer size or layout require +the stream to be stopped. Any attempt at such a modification while the stream +is active shall cause the ioctl setting the format or the control to return +the ``EBUSY`` error code. This is my problem with putting the more complex case first: if you are reading this for the first time then the preceding paragraph is simply *wrong*. You cannot modify the buffer size when the stream is stopped. You need to free all buffers first before you can do that. That's driver-dependent. If I reverse the order to start with the more restricted case, it will also be wrong for drivers that implement the other case. It's not about wrong or right, it's about which case is the most common. > I'm running out of time so I'll change this though. We can always reorder the paragraphs later when we will have more drivers implementing the more powerful case. Unless the driver has been especially written to allow that. And I am not aware of any. + +Controls that only influence the buffer layout can be modified at any time +when the stream is stopped. As they don't influence the buffer size, no +special handling is needed to synchronize those controls with buffer +allocation. + +Formats and controls that influence the buffer size interact with buffer +allocation.
Re: [PATCH v2.2] v4l: Clearly document interactions between formats, controls and buffers
Hi Hans, On Monday 06 Mar 2017 11:04:50 Hans Verkuil wrote: > On 05/03/17 22:36, Laurent Pinchart wrote: > > V4L2 exposes parameters that influence buffers sizes through the format > > ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT, VIDIOC_S_FMT, and possibly > > VIDIOC_G_SELECTION and VIDIOC_S_SELECTION). Other parameters not part of > > the format structure may also influence buffer sizes or buffer layout in > > general. One existing such parameter is rotation, which is implemented > > by the V4L2_CID_ROTATE control and thus exposed through the V4L2 control > > ioctls. > > > > The interaction between those parameters and buffers is currently only > > partially specified by the V4L2 API. In particular interactions between > > controls and buffers isn't specified at all. The behaviour of the > > VIDIOC_S_FMT and VIDIOC_S_SELECTION ioctls when buffers are allocated is > > also not fully specified. > > > > This patch clearly defines and documents the interactions between > > formats, selections, controls and buffers. > > > > The preparatory discussions for the documentation change considered > > completely disallowing controls that change the buffer size or layout, > > in favour of extending the format API with a new ioctl that would bundle > > those controls with format information. The idea has been rejected, as > > this would essentially be a restricted version of the upcoming request > > API that wouldn't bring any additional value. > > > > Another option we have considered was to mandate the use of the request > > API to modify controls that influence buffer size or layout. This has > > also been rejected on the grounds that requiring the request API to > > change rotation even when streaming is stopped would significantly > > complicate implementation of drivers and usage of the V4L2 API for > > applications. > > > > Applications will however be required to use the upcoming request API to > > change at runtime formats or controls that influence the buffer size or > > layout, because of the need to synchronize buffers with the formats and > > controls. Otherwise there would be no way to interpret the content of a > > buffer correctly. > > > > Signed-off-by: Laurent Pinchart > > > > Acked-by: Sakari Ailus > > --- > > Changes since v2.1: > > > > - Fixed small issues in commit message > > - Simplified wording of one sentence in the documentation > > > > Changes since v2: > > > > - Document the interaction with ioctls that can affect formats > > (VIDIOC_S_SELECTION, VIDIOC_S_INPUT, VIDIOC_S_OUTPUT, VIDIOC_S_STD and > > VIDIOC_S_DV_TIMINGS) > > - Clarify the format/control change order > > --- > > > > Documentation/media/uapi/v4l/buffer.rst | 108 +++ > > 1 file changed, 108 insertions(+) > > > > diff --git a/Documentation/media/uapi/v4l/buffer.rst > > b/Documentation/media/uapi/v4l/buffer.rst index > > ac58966ccb9b..60d62a5824f8 100644 > > --- a/Documentation/media/uapi/v4l/buffer.rst > > +++ b/Documentation/media/uapi/v4l/buffer.rst > > @@ -34,6 +34,114 @@ flags are copied from the OUTPUT video buffer to the > > CAPTURE video> > > buffer. > > > > +Interactions between formats, controls and buffers > > +== > > + > > +V4L2 exposes parameters that influence the buffer size, or the way data > > is +laid out in the buffer. Those parameters are exposed through both > > formats and +controls. One example of such a control is the > > ``V4L2_CID_ROTATE`` control +that modifies the direction in which pixels > > are stored in the buffer, as well +as the buffer size when the selected > > format includes padding at the end of +lines. > > + > > +The set of information needed to interpret the content of a buffer (e.g. > > the +pixel format, the line stride, the tiling orientation or the > > rotation) is +collectively referred to in the rest of this section as the > > buffer layout. > > + > > +Modifying formats or controls that influence the buffer size or layout > > require +the stream to be stopped. Any attempt at such a modification > > while the stream +is active shall cause the ioctl setting the format or > > the control to return +the ``EBUSY`` error code. > > This is my problem with putting the more complex case first: if you are > reading this for the first time then the preceding paragraph is simply > *wrong*. > > You cannot modify the buffer size when the stream is stopped. You need to > free all buffers first before you can do that. That's driver-dependent. If I reverse the order to start with the more restricted case, it will also be wrong for drivers that implement the other case. I'm running out of time so I'll change this though. We can always reorder the paragraphs later when we will have more drivers implementing the more powerful case. > Unless the driver has been especially written to allow that. And I am not > aware of any. > > > + > > +Controls that only influence the buffer layout can be modified at any > > t
Re: [PATCH v2.2] v4l: Clearly document interactions between formats, controls and buffers
On 05/03/17 22:36, Laurent Pinchart wrote: V4L2 exposes parameters that influence buffers sizes through the format ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT, VIDIOC_S_FMT, and possibly VIDIOC_G_SELECTION and VIDIOC_S_SELECTION). Other parameters not part of the format structure may also influence buffer sizes or buffer layout in general. One existing such parameter is rotation, which is implemented by the V4L2_CID_ROTATE control and thus exposed through the V4L2 control ioctls. The interaction between those parameters and buffers is currently only partially specified by the V4L2 API. In particular interactions between controls and buffers isn't specified at all. The behaviour of the VIDIOC_S_FMT and VIDIOC_S_SELECTION ioctls when buffers are allocated is also not fully specified. This patch clearly defines and documents the interactions between formats, selections, controls and buffers. The preparatory discussions for the documentation change considered completely disallowing controls that change the buffer size or layout, in favour of extending the format API with a new ioctl that would bundle those controls with format information. The idea has been rejected, as this would essentially be a restricted version of the upcoming request API that wouldn't bring any additional value. Another option we have considered was to mandate the use of the request API to modify controls that influence buffer size or layout. This has also been rejected on the grounds that requiring the request API to change rotation even when streaming is stopped would significantly complicate implementation of drivers and usage of the V4L2 API for applications. Applications will however be required to use the upcoming request API to change at runtime formats or controls that influence the buffer size or layout, because of the need to synchronize buffers with the formats and controls. Otherwise there would be no way to interpret the content of a buffer correctly. Signed-off-by: Laurent Pinchart Acked-by: Sakari Ailus --- Changes since v2.1: - Fixed small issues in commit message - Simplified wording of one sentence in the documentation Changes since v2: - Document the interaction with ioctls that can affect formats (VIDIOC_S_SELECTION, VIDIOC_S_INPUT, VIDIOC_S_OUTPUT, VIDIOC_S_STD and VIDIOC_S_DV_TIMINGS) - Clarify the format/control change order --- Documentation/media/uapi/v4l/buffer.rst | 108 1 file changed, 108 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ac58966ccb9b..60d62a5824f8 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -34,6 +34,114 @@ flags are copied from the OUTPUT video buffer to the CAPTURE video buffer. +Interactions between formats, controls and buffers +== + +V4L2 exposes parameters that influence the buffer size, or the way data is +laid out in the buffer. Those parameters are exposed through both formats and +controls. One example of such a control is the ``V4L2_CID_ROTATE`` control +that modifies the direction in which pixels are stored in the buffer, as well +as the buffer size when the selected format includes padding at the end of +lines. + +The set of information needed to interpret the content of a buffer (e.g. the +pixel format, the line stride, the tiling orientation or the rotation) is +collectively referred to in the rest of this section as the buffer layout. + +Modifying formats or controls that influence the buffer size or layout require +the stream to be stopped. Any attempt at such a modification while the stream +is active shall cause the ioctl setting the format or the control to return +the ``EBUSY`` error code. This is my problem with putting the more complex case first: if you are reading this for the first time then the preceding paragraph is simply *wrong*. You cannot modify the buffer size when the stream is stopped. You need to free all buffers first before you can do that. Unless the driver has been especially written to allow that. And I am not aware of any. + +Controls that only influence the buffer layout can be modified at any time +when the stream is stopped. As they don't influence the buffer size, no +special handling is needed to synchronize those controls with buffer +allocation. + +Formats and controls that influence the buffer size interact with buffer +allocation. As buffer allocation is an expensive operation, drivers should +allow format or controls that influence the buffer size to be changed with +buffers allocated. A typical ioctl sequence to modify format and controls is + + #. VIDIOC_STREAMOFF + #. VIDIOC_S_EXT_CTRLS + #. VIDIOC_S_FMT + #. VIDIOC_QBUF + #. VIDIOC_STREAMON + +.. note:: + + The API doesn't mandate the above order for control (2.) and format (3.) + changes. Format and controls can be set in a different order, or even + interle
[PATCH v2.2] v4l: Clearly document interactions between formats, controls and buffers
V4L2 exposes parameters that influence buffers sizes through the format ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT, VIDIOC_S_FMT, and possibly VIDIOC_G_SELECTION and VIDIOC_S_SELECTION). Other parameters not part of the format structure may also influence buffer sizes or buffer layout in general. One existing such parameter is rotation, which is implemented by the V4L2_CID_ROTATE control and thus exposed through the V4L2 control ioctls. The interaction between those parameters and buffers is currently only partially specified by the V4L2 API. In particular interactions between controls and buffers isn't specified at all. The behaviour of the VIDIOC_S_FMT and VIDIOC_S_SELECTION ioctls when buffers are allocated is also not fully specified. This patch clearly defines and documents the interactions between formats, selections, controls and buffers. The preparatory discussions for the documentation change considered completely disallowing controls that change the buffer size or layout, in favour of extending the format API with a new ioctl that would bundle those controls with format information. The idea has been rejected, as this would essentially be a restricted version of the upcoming request API that wouldn't bring any additional value. Another option we have considered was to mandate the use of the request API to modify controls that influence buffer size or layout. This has also been rejected on the grounds that requiring the request API to change rotation even when streaming is stopped would significantly complicate implementation of drivers and usage of the V4L2 API for applications. Applications will however be required to use the upcoming request API to change at runtime formats or controls that influence the buffer size or layout, because of the need to synchronize buffers with the formats and controls. Otherwise there would be no way to interpret the content of a buffer correctly. Signed-off-by: Laurent Pinchart Acked-by: Sakari Ailus --- Changes since v2.1: - Fixed small issues in commit message - Simplified wording of one sentence in the documentation Changes since v2: - Document the interaction with ioctls that can affect formats (VIDIOC_S_SELECTION, VIDIOC_S_INPUT, VIDIOC_S_OUTPUT, VIDIOC_S_STD and VIDIOC_S_DV_TIMINGS) - Clarify the format/control change order --- Documentation/media/uapi/v4l/buffer.rst | 108 1 file changed, 108 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ac58966ccb9b..60d62a5824f8 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -34,6 +34,114 @@ flags are copied from the OUTPUT video buffer to the CAPTURE video buffer. +Interactions between formats, controls and buffers +== + +V4L2 exposes parameters that influence the buffer size, or the way data is +laid out in the buffer. Those parameters are exposed through both formats and +controls. One example of such a control is the ``V4L2_CID_ROTATE`` control +that modifies the direction in which pixels are stored in the buffer, as well +as the buffer size when the selected format includes padding at the end of +lines. + +The set of information needed to interpret the content of a buffer (e.g. the +pixel format, the line stride, the tiling orientation or the rotation) is +collectively referred to in the rest of this section as the buffer layout. + +Modifying formats or controls that influence the buffer size or layout require +the stream to be stopped. Any attempt at such a modification while the stream +is active shall cause the ioctl setting the format or the control to return +the ``EBUSY`` error code. + +Controls that only influence the buffer layout can be modified at any time +when the stream is stopped. As they don't influence the buffer size, no +special handling is needed to synchronize those controls with buffer +allocation. + +Formats and controls that influence the buffer size interact with buffer +allocation. As buffer allocation is an expensive operation, drivers should +allow format or controls that influence the buffer size to be changed with +buffers allocated. A typical ioctl sequence to modify format and controls is + + #. VIDIOC_STREAMOFF + #. VIDIOC_S_EXT_CTRLS + #. VIDIOC_S_FMT + #. VIDIOC_QBUF + #. VIDIOC_STREAMON + +.. note:: + + The API doesn't mandate the above order for control (2.) and format (3.) + changes. Format and controls can be set in a different order, or even + interleaved, depending on the device and use case. For instance some + controls might behave differently for different pixel formats, in which + case the format might need to be set first. + +Queued buffers must be large enough for the new format or controls. + +Drivers shall return a ``ENOSPC`` error in response to format change +(:c:func:`VIDIOC_S_FMT`) or control changes (:c:func:`VIDIOC_S_CTRL` or +:c:func
