Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
On 05/03/17 15:35, Laurent Pinchart wrote: Hi Hans, On Saturday 04 Mar 2017 11:53:45 Hans Verkuil wrote: On 28/02/17 16:03, Laurent Pinchart wrote: V4L2 exposes parameters that influence buffers sizes through the format ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). Other parameters S_SELECTION should be mentioned here as well (more about that later). I'll update that according to the conversation we had on IRC about this topic. 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 VIDIOC_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 ioctl when buffers are allocated is also not fully specified. This commit clearly defines and documents the interactions between formats, 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 --- Documentation/media/uapi/v4l/buffer.rst | 88 1 file changed, 88 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ac58966ccb9b..5c58db98ab7a 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -34,6 +34,94 @@ 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 format or control set ioctl to return the ``EBUSY`` +error code. This is not what happens today: it's not the streaming part that causes EBUSY to be returned but whether or not buffers are allocated. Note that this patch documents the recommended behaviour we want to achieve. It can differ from the behaviour implemented today as long as today's behaviour (at least the behaviour of the drivers that we don't consider as non-compliant, which is the vast majority) is not contradicted by this patch. In other words, we can recommend here a more generic versatile and powerful behaviour as long as the more restricted behaviour implemented today is also allowed. Today we do not support changing buffer sizes on the fly, so any attempt to call an ioctl that would change the buffer size is blocked and EBUSY is returned. To be precise: drivers call vb2_is_busy() to determine this. To my knowledge all vb2-using drivers behave like this. There may be old drivers that do not do this (and these have a high likelyhood of being wrong). Please also note that the above description doesn't contradict the behaviour implemented today. It explains that changing formats and controls is not allowed while streaming, which is a subset of the restriction implemented by many drivers through vb2_is_busy() of not allowing changes when buffers are allocated. Please see below for more about this in response to your "here's the 99% case" comment. + +Controls that only
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
On 04/03/17 15:37, Sakari Ailus wrote: Hi Hans (and Laurent). On Sat, Mar 04, 2017 at 11:53:45AM +0100, Hans Verkuil wrote: Hi Laurent, Here is my review: On 28/02/17 16:03, Laurent Pinchart wrote: V4L2 exposes parameters that influence buffers sizes through the format ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). Other parameters S_SELECTION should be mentioned here as well (more about that later). 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 VIDIOC_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 ioctl when buffers are allocated is also not fully specified. This commit clearly defines and documents the interactions between formats, 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 --- Documentation/media/uapi/v4l/buffer.rst | 88 + 1 file changed, 88 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ac58966ccb9b..5c58db98ab7a 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -34,6 +34,94 @@ 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 format or control set ioctl to return the ``EBUSY`` +error code. This is not what happens today: it's not the streaming part that causes EBUSY to be returned but whether or not buffers are allocated. Today we do not support changing buffer sizes on the fly, so any attempt to call an ioctl that would change the buffer size is blocked and EBUSY is returned. To be precise: drivers call vb2_is_busy() to determine this. It certainly shouldn't be like that. Not allowing S_FMT() while there are buffers allocated makes CREATE_BUFS entirely useless. Well, CREATE_BUFS can still be used to make large buffers in which the image is composed. But yes, CREATE_BUFS has limited usefulness today, but the simple fact is that nobody wrote drivers that really can do on the fly format changes. To my knowledge all vb2-using drivers behave like this. There may be old drivers that do not do this (and these have a high likelyhood of being wrong). What's really needed is that the driver verifies that the buffer is large enough to be used for a given format. vb2_is_busy() shouldn't be used to check whether setting format is allowed. Again, there are no drivers that support this. Using vb2_is_busy() prevents userspace from trying this, since without proper driver support this *will* fail. Of course, drivers that do support this will not test vb2_is_busy but the vb2_is_streaming instead (and with the request API even that can be dropped). + +Controls that only influence the b
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
Hi Hans, On Saturday 04 Mar 2017 11:53:45 Hans Verkuil wrote: > On 28/02/17 16:03, Laurent Pinchart wrote: > > V4L2 exposes parameters that influence buffers sizes through the format > > ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). Other parameters > > S_SELECTION should be mentioned here as well (more about that later). I'll update that according to the conversation we had on IRC about this topic. > > 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 VIDIOC_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 ioctl when buffers are allocated is also not fully > > specified. > > > > This commit clearly defines and documents the interactions between > > formats, 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 > > > > --- > > > > Documentation/media/uapi/v4l/buffer.rst | 88 > > 1 file changed, 88 insertions(+) > > > > diff --git a/Documentation/media/uapi/v4l/buffer.rst > > b/Documentation/media/uapi/v4l/buffer.rst index > > ac58966ccb9b..5c58db98ab7a 100644 > > --- a/Documentation/media/uapi/v4l/buffer.rst > > +++ b/Documentation/media/uapi/v4l/buffer.rst > > @@ -34,6 +34,94 @@ 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 format or control set ioctl > > to return the ``EBUSY`` +error code. > > This is not what happens today: it's not the streaming part that causes > EBUSY to be returned but whether or not buffers are allocated. Note that this patch documents the recommended behaviour we want to achieve. It can differ from the behaviour implemented today as long as today's behaviour (at least the behaviour of the drivers that we don't consider as non-compliant, which is the vast majority) is not contradicted by this patch. In other words, we can recommend here a more generic versatile and powerful behaviour as long as the more restricted behaviour implemented today is also allowed. > Today we do not support changing buffer sizes on the fly, so any attempt to > call an ioctl that would change the buffer size is blocked and EBUSY is > returned. To be precise: drivers call vb2_is_busy() to determine this. > > To my knowledge all vb2-using drivers behave like this. There may be old > drivers that do not do this (and these have a high likelyhood of being > wrong). Please also note that the above description doesn't contradict the behaviour implemented today. It explains that changing formats and controls is not allowed whi
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
Hi Sakari, On Saturday 04 Mar 2017 15:48:54 Sakari Ailus wrote: > On Sat, Mar 04, 2017 at 11:57:32AM +0100, Hans Verkuil wrote: > ... > > >>> +To simplify their implementation, drivers may also require buffers to > >>> be +reallocated in order to change formats or controls that influence > >>> the buffer +size. In that case, to perform such changes, userspace > >>> applications shall +first stop the video stream with the > >>> :c:func:`VIDIOC_STREAMOFF` ioctl if it +is running and free all buffers > >>> with the :c:func:`VIDIOC_REQBUFS` ioctl if +they are allocated. The > >>> format or controls can then be modified, and buffers +shall then be > >>> reallocated and the stream restarted. A typical ioctl sequence +is > >>> + > >>> + #. VIDIOC_STREAMOFF > >>> + #. VIDIOC_REQBUFS(0) > >>> + #. VIDIOC_S_FMT > >>> + #. VIDIOC_S_EXT_CTRLS > >> > >> Same here. > >> > >> Would it be safe to say that controls are changed first? I wonder if > >> there could be special cases where this wouldn't apply though. It could > >> ultimately come down to hardware features: rotation might be only > >> available for certain formats so you'd need to change the format first > >> to enable rotation. > >> > >> What you're documenting above is a typical sequence so it doesn't have to > >> be applicable to all potential hardware. I might mention there could be > >> such dependencies. I wonder if one exists at the moment. No? > > > > The way V4L2 works is that the last ioctl called gets 'preference'. So the > > driver should attempt to satisfy the ioctl, even if that means undoing > > previous ioctls. In other words, V4L2 allows any order, but the > > end-result might be different depending on the hardware capabilities. > > Indeed. But the above sequence suggests that formats are set before > controls. I suggested to clarify that part. I agree with both of you. I'll clarify that this is just an example and that formats and controls can be set in a different order (or even interleaved). -- Regards, Laurent Pinchart
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
Hi Hans (and Laurent). On Sat, Mar 04, 2017 at 11:53:45AM +0100, Hans Verkuil wrote: > Hi Laurent, > > Here is my review: > > On 28/02/17 16:03, Laurent Pinchart wrote: > >V4L2 exposes parameters that influence buffers sizes through the format > >ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). Other parameters > > S_SELECTION should be mentioned here as well (more about that later). > > >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 VIDIOC_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 ioctl when buffers are allocated is also not fully > >specified. > > > >This commit clearly defines and documents the interactions between > >formats, 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 > >--- > > Documentation/media/uapi/v4l/buffer.rst | 88 > > + > > 1 file changed, 88 insertions(+) > > > >diff --git a/Documentation/media/uapi/v4l/buffer.rst > >b/Documentation/media/uapi/v4l/buffer.rst > >index ac58966ccb9b..5c58db98ab7a 100644 > >--- a/Documentation/media/uapi/v4l/buffer.rst > >+++ b/Documentation/media/uapi/v4l/buffer.rst > >@@ -34,6 +34,94 @@ 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 format or control set ioctl to return the > >``EBUSY`` > >+error code. > > This is not what happens today: it's not the streaming part that causes EBUSY > to > be returned but whether or not buffers are allocated. > > Today we do not support changing buffer sizes on the fly, so any attempt to > call an ioctl that would change the buffer size is blocked and EBUSY is > returned. > To be precise: drivers call vb2_is_busy() to determine this. It certainly shouldn't be like that. Not allowing S_FMT() while there are buffers allocated makes CREATE_BUFS entirely useless. > > To my knowledge all vb2-using drivers behave like this. There may be old > drivers > that do not do this (and these have a high likelyhood of being wrong). What's really needed is that the driver verifies that the buffer is large enough to be used for a given format. vb2_is_busy() shouldn't be used to check whether setting format is allowed. > > >+ > >+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 alloc
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
Hi Hans, On Sat, Mar 04, 2017 at 11:57:32AM +0100, Hans Verkuil wrote: ... > >>+To simplify their implementation, drivers may also require buffers to be > >>+reallocated in order to change formats or controls that influence the > >>buffer > >>+size. In that case, to perform such changes, userspace applications shall > >>+first stop the video stream with the :c:func:`VIDIOC_STREAMOFF` ioctl if it > >>+is running and free all buffers with the :c:func:`VIDIOC_REQBUFS` ioctl if > >>+they are allocated. The format or controls can then be modified, and > >>buffers > >>+shall then be reallocated and the stream restarted. A typical ioctl > >>sequence > >>+is > >>+ > >>+ #. VIDIOC_STREAMOFF > >>+ #. VIDIOC_REQBUFS(0) > >>+ #. VIDIOC_S_FMT > >>+ #. VIDIOC_S_EXT_CTRLS > > > >Same here. > > > >Would it be safe to say that controls are changed first? I wonder if there > >could be special cases where this wouldn't apply though. It could ultimately > >come down to hardware features: rotation might be only available for certain > >formats so you'd need to change the format first to enable rotation. > > > >What you're documenting above is a typical sequence so it doesn't have to be > >applicable to all potential hardware. I might mention there could be such > >dependencies. I wonder if one exists at the moment. No? > > The way V4L2 works is that the last ioctl called gets 'preference'. So the > driver should attempt to satisfy the ioctl, even if that means undoing > previous > ioctls. In other words, V4L2 allows any order, but the end-result might be > different depending on the hardware capabilities. Indeed. But the above sequence suggests that formats are set before controls. I suggested to clarify that part. -- Regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
On 02/03/17 16:37, Sakari Ailus wrote: Hi Laurent, On Tue, Feb 28, 2017 at 05:03:19PM +0200, Laurent Pinchart wrote: V4L2 exposes parameters that influence buffers sizes through the format ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). 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 VIDIOC_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 ioctl when buffers are allocated is also not fully specified. This commit clearly defines and documents the interactions between formats, 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 --- Documentation/media/uapi/v4l/buffer.rst | 88 + 1 file changed, 88 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ac58966ccb9b..5c58db98ab7a 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -34,6 +34,94 @@ 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_CI, and also documented in the documentation of those ioctls (if that isn't done already). The latter can be done later, or D_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 format or control set ioctl 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_FMT + #. VIDIOC_S_EXT_CTRLS Which one do you set first, the format or the controls? Supposedly the user would have to get the format again after setting the ROTATE control. + #. VIDIOC_QBUF + #. VIDIOC_STREAMON + +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:`VIDIOC_S_EXT_CTRLS`) if buffers too small for the new format are +currently queued. As a simplification, drivers are allowed to return an error +from these ioctls if any buffer is currently queued, without checking the +queued buffers sizes. Drivers shall also return a ``ENOSPC`` error from the +:c:func:`VIDIOC_QBUF` ioctl if the buffer being queued is too small for the +current format or controls. Together, these requirements ensure t
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
Hi Laurent, Here is my review: On 28/02/17 16:03, Laurent Pinchart wrote: V4L2 exposes parameters that influence buffers sizes through the format ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). Other parameters S_SELECTION should be mentioned here as well (more about that later). 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 VIDIOC_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 ioctl when buffers are allocated is also not fully specified. This commit clearly defines and documents the interactions between formats, 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 --- Documentation/media/uapi/v4l/buffer.rst | 88 + 1 file changed, 88 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ac58966ccb9b..5c58db98ab7a 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -34,6 +34,94 @@ 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 format or control set ioctl to return the ``EBUSY`` +error code. This is not what happens today: it's not the streaming part that causes EBUSY to be returned but whether or not buffers are allocated. Today we do not support changing buffer sizes on the fly, so any attempt to call an ioctl that would change the buffer size is blocked and EBUSY is returned. To be precise: drivers call vb2_is_busy() to determine this. To my knowledge all vb2-using drivers behave like this. There may be old drivers that do not do this (and these have a high likelyhood of being wrong). + +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_FMT + #. VIDIOC_S_EXT_CTRLS + #. VIDIOC_QBUF + #. VIDIOC_STREAMON + +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:`VIDIOC_S_EXT_CTRLS`) if buffers too small for the new format are +currently queued. As a simplification, drivers are allowed to return an error +from these ioctls if any
Re: [PATCH v2 2/3] v4l: Clearly document interactions between formats, controls and buffers
Hi Laurent, On Tue, Feb 28, 2017 at 05:03:19PM +0200, Laurent Pinchart wrote: > V4L2 exposes parameters that influence buffers sizes through the format > ioctls (VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIO_S_FMT). 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 VIDIOC_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 ioctl when buffers are allocated is also not fully > specified. > > This commit clearly defines and documents the interactions between > formats, 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 > --- > Documentation/media/uapi/v4l/buffer.rst | 88 > + > 1 file changed, 88 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/buffer.rst > b/Documentation/media/uapi/v4l/buffer.rst > index ac58966ccb9b..5c58db98ab7a 100644 > --- a/Documentation/media/uapi/v4l/buffer.rst > +++ b/Documentation/media/uapi/v4l/buffer.rst > @@ -34,6 +34,94 @@ 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 format or control set ioctl 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_FMT > + #. VIDIOC_S_EXT_CTRLS Which one do you set first, the format or the controls? Supposedly the user would have to get the format again after setting the ROTATE control. > + #. VIDIOC_QBUF > + #. VIDIOC_STREAMON > + > +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:`VIDIOC_S_EXT_CTRLS`) if buffers too small for the new format are > +currently queued. As a simplification, drivers are allowed to return an error > +from these ioctls if any buffer is currently queued, without checking the > +queued buffers sizes. Drivers shall also return a ``ENOSPC`` error from the > +:c:func:`VIDIOC_QBUF` ioctl if the buffer being queued is too small for the > +current format or controls. Togethe
[PATCH v2 2/3] 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 and VIDIO_S_FMT). 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 VIDIOC_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 ioctl when buffers are allocated is also not fully specified. This commit clearly defines and documents the interactions between formats, 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 --- Documentation/media/uapi/v4l/buffer.rst | 88 + 1 file changed, 88 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ac58966ccb9b..5c58db98ab7a 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -34,6 +34,94 @@ 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 format or control set ioctl 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_FMT + #. VIDIOC_S_EXT_CTRLS + #. VIDIOC_QBUF + #. VIDIOC_STREAMON + +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:`VIDIOC_S_EXT_CTRLS`) if buffers too small for the new format are +currently queued. As a simplification, drivers are allowed to return an error +from these ioctls if any buffer is currently queued, without checking the +queued buffers sizes. Drivers shall also return a ``ENOSPC`` error from the +:c:func:`VIDIOC_QBUF` ioctl if the buffer being queued is too small for the +current format or controls. Together, these requirements ensure that queued +buffers will always be large enough for the configured format and controls. + +Userspace applications can query the buffer size required for a given format +and controls by first setting the desired control values and then trying the +desired format. The :c:func:`VIDIOC_TRY_FMT` ioctl will return the required +buffer size. + + #. VIDIOC_S_EXT_CTRLS(x) + #. VIDIOC_TRY_FMT() + #. V
