From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from lb3-smtp-cloud2.xs4all.net ([194.109.24.29]:38114 "EHLO lb3-smtp-cloud2.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752726AbbEDK3g (ORCPT ); Mon, 4 May 2015 06:29:36 -0400 Message-ID: <55474A05.2040405@xs4all.nl> Date: Mon, 04 May 2015 12:29:25 +0200 From: Hans Verkuil MIME-Version: 1.0 To: Philipp Zabel CC: linux-media@vger.kernel.org, Pawel Osciak , Kamil Debski , Laurent Pinchart , Nicolas Dufresne , Sakari Ailus , kernel@pengutronix.de Subject: Re: [PATCH v5 1/5] [media] DocBook media: document mem2mem draining flow References: <1429518504-14880-1-git-send-email-p.zabel@pengutronix.de> <1429518504-14880-2-git-send-email-p.zabel@pengutronix.de> <553E3836.80606@xs4all.nl> <1430734901.3722.30.camel@pengutronix.de> In-Reply-To: <1430734901.3722.30.camel@pengutronix.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: On 05/04/2015 12:21 PM, Philipp Zabel wrote: > 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 >>> --- >>> 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. >>> >>> >>> + V4L2_BUF_FLAG_LAST >>> + 0x00100000 >>> + Last buffer produced by the hardware. mem2mem codec drivers >>> +set this flag on the capture queue for the last buffer when the >>> +VIDIOC_QUERYBUF or >>> +VIDIOC_DQBUF ioctl is called. Any subsequent >>> +call to the VIDIOC_DQBUF ioctl will not block >>> +anymore, but return an &EPIPE;. >> >> 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. > Last buffer produced by the hardware. mem2mem codec drivers > set this flag on the capture queue for the last buffer when the > VIDIOC_QUERYBUF or > -VIDIOC_DQBUF ioctl is called. Any subsequent > -call to the VIDIOC_DQBUF ioctl will not block > -anymore, but return an &EPIPE;. > +VIDIOC_DQBUF ioctl is called. Due to hardware > +limitations, the last buffer may be empty. In this case the driver will set the > +bytesused field to 0, regardless of the format. Any > +Any subsequent call to the VIDIOC_DQBUF ioctl > +will not block anymore, but return an &EPIPE;. > > > V4L2_BUF_FLAG_TIMESTAMP_MASK > >>> + >>> + >>> V4L2_BUF_FLAG_TIMESTAMP_MASK >>> 0x0000e000 >>> 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 V4L2_DEC_CMD_STOP_TO_BLACK is set, then the decoder will >>> set the picture to black after it stopped decoding. Otherwise the last image will >>> -repeat. If V4L2_DEC_CMD_STOP_IMMEDIATELY is set, then the decoder >>> +repeat. mem2mem decoders will stop producing new frames altogether. They will send >>> +a V4L2_EVENT_EOS event when the last frame has been decoded >>> +and all frames are ready to be dequeued and will set the >>> +V4L2_BUF_FLAG_LAST 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 V4L2_EVENT_EOS event when the last frame has been decoded > and all frames are ready to be dequeued and will set the > V4L2_BUF_FLAG_LAST 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 VIDIOC_DQBUF 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 > +bytesused field to 0. Once the > +V4L2_BUF_FLAG_LAST flag was set, the > +VIDIOC_DQBUF ioctl will not block anymore, > +but return an &EPIPE;. > If V4L2_DEC_CMD_STOP_IMMEDIATELY is set, then the decoder > stops immediately (ignoring the pts 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 VIDIOC_DQBUF ioctl >>> +will not block anymore, but return an &EPIPE;. >>> +If V4L2_DEC_CMD_STOP_IMMEDIATELY is set, then the decoder >>> stops immediately (ignoring the pts 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. >>> encoding will continue until the end of the current Group >>> Of Pictures, otherwise encoding will stop immediately. >>> When the encoder is already stopped, this command does >>> -nothing. >>> +nothing. mem2mem encoders will send a V4L2_EVENT_EOS event >>> +when the last frame has been decoded and all frames are ready to be dequeued and >>> +will set the V4L2_BUF_FLAG_LAST 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 V4L2_EVENT_EOS event > when the last frame has been decoded and all frames are ready to be dequeued and > will set the V4L2_BUF_FLAG_LAST 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 > +bytesused field to 0. Once the > +V4L2_BUF_FLAG_LAST flag was set, the > VIDIOC_DQBUF ioctl will not block anymore, > but return an &EPIPE;. > Looks good to me. With this change: Acked-by: Hans Verkuil Regards, Hans > > regards > Philipp > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >