Hi Hans, Kamil,
thank you for your comments.
Am Montag, den 27.04.2015, 15:23 +0200 schrieb Hans Verkuil:
> Hi Philipp,
>
> I finally got around to reviewing this patch series. Sorry for the delay, but
> here are my comments:
>
> On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> > Document the interaction between VIDIOC_DECODER_CMD V4L2_DEC_CMD_STOP and
> > VIDIOC_ENCODER_CMD V4L2_ENC_CMD_STOP to start the draining, the V4L2_EVENT_EOS
> > event signalling all capture buffers are finished and ready to be dequeud,
> > the new V4L2_BUF_FLAG_LAST buffer flag indicating the last buffer being dequeued
> > from the capture queue, and the poll and VIDIOC_DQBUF ioctl return values once
> > the queue is drained.
> >
> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > ---
> > Changes since v4:
> > - Split out documentation changes into a separate patch
> > - Changed wording following Pawel's suggestions.
> > ---
> > Documentation/DocBook/media/v4l/io.xml | 10 ++++++++++
> > Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml | 9 ++++++++-
> > Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml | 8 +++++++-
> > Documentation/DocBook/media/v4l/vidioc-qbuf.xml | 8 ++++++++
> > 4 files changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> > index 1c17f80..f561037 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -1129,6 +1129,16 @@ in this buffer has not been created by the CPU but by some DMA-capable unit,
> > in which case caches have not been used.</entry>
> > </row>
> > <row>
> > + <entry><constant>V4L2_BUF_FLAG_LAST</constant></entry>
> > + <entry>0x00100000</entry>
> > + <entry>Last buffer produced by the hardware. mem2mem codec drivers
> > +set this flag on the capture queue for the last buffer when the
> > +<link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
> > +<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
> > +call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
> > +anymore, but return an &EPIPE;.</entry>
>
> As Kamil mentioned in his review, we should allow for bytesused == 0 here.
How about:
@@ -1134,9 +1134,11 @@ in which case caches have not been used.</entry>
<entry>Last buffer produced by the hardware. mem2mem codec drivers
set this flag on the capture queue for the last buffer when the
<link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> or
-<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Any subsequent
-call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block
-anymore, but return an &EPIPE;.</entry>
+<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called. Due to hardware
+limitations, the last buffer may be empty. In this case the driver will set the
+<structfield>bytesused</structfield> field to 0, regardless of the format. Any
+Any subsequent call to the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
+will not block anymore, but return an &EPIPE;.</entry>
</row>
<row>
<entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
> > + </row>
> > + <row>
> > <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
> > <entry>0x0000e000</entry>
> > <entry>Mask for timestamp types below. To test the
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> > index 9215627..6502d82 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> > @@ -197,7 +197,14 @@ be muted when playing back at a non-standard speed.
> > this command does nothing. This command has two flags:
> > if <constant>V4L2_DEC_CMD_STOP_TO_BLACK</constant> is set, then the decoder will
> > set the picture to black after it stopped decoding. Otherwise the last image will
> > -repeat. If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
> > +repeat. mem2mem decoders will stop producing new frames altogether. They will send
> > +a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
> > +and all frames are ready to be dequeued and will set the
> > +<constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the
>
> Make a note here as well that the last buffer might be an empty buffer.
@@ -201,9 +201,12 @@ repeat. mem2mem decoders will stop producing new frames altogether. They will se
a <constant>V4L2_EVENT_EOS</constant> event when the last frame has been decoded
and all frames are ready to be dequeued and will set the
<constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last buffer of the
-capture queue to indicate there will be no new buffers produced to dequeue. Once
-this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
-will not block anymore, but return an &EPIPE;.
+capture queue to indicate there will be no new buffers produced to dequeue. This
+buffer may be empty, indicated by the driver setting the
+<structfield>bytesused</structfield> field to 0. Once the
+<constant>V4L2_BUF_FLAG_LAST</constant> flag was set, the
+<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
+but return an &EPIPE;.
If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
will keep decoding until timestamp >= pts or until the last of the pending data from
> > +capture queue to indicate there will be no new buffers produced to dequeue. Once
> > +this flag was set, the <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl
> > +will not block anymore, but return an &EPIPE;.
> > +If <constant>V4L2_DEC_CMD_STOP_IMMEDIATELY</constant> is set, then the decoder
> > stops immediately (ignoring the <structfield>pts</structfield> value), otherwise it
> > will keep decoding until timestamp >= pts or until the last of the pending data from
> > its internal buffers was decoded.
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> > index 0619ca5..3cdb841 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> > @@ -129,7 +129,13 @@ this command.</entry>
> > encoding will continue until the end of the current <wordasword>Group
> > Of Pictures</wordasword>, otherwise encoding will stop immediately.
> > When the encoder is already stopped, this command does
> > -nothing.</entry>
> > +nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
> > +when the last frame has been decoded and all frames are ready to be dequeued and
> > +will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
> > +buffer of the capture queue to indicate there will be no new buffers produced to
>
> Ditto.
@@ -133,7 +133,9 @@ nothing. mem2mem encoders will send a <constant>V4L2_EVENT_EOS</constant> event
when the last frame has been decoded and all frames are ready to be dequeued and
will set the <constant>V4L2_BUF_FLAG_LAST</constant> buffer flag on the last
buffer of the capture queue to indicate there will be no new buffers produced to
-dequeue. Once this flag was set, the
+dequeue. This buffer may be empty, indicated by the driver setting the
+<structfield>bytesused</structfield> field to 0. Once the
+<constant>V4L2_BUF_FLAG_LAST</constant> flag was set, the
<link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl will not block anymore,
but return an &EPIPE;.</entry>
</row>
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
