Le vendredi 19 octobre 2018 à 07:35 -0400, Nicolas Dufresne a écrit :
> Le vendredi 19 octobre 2018 à 09:28 +0200, Hans Verkuil a écrit :
> > On 10/18/2018 06:08 PM, Ezequiel Garcia wrote:
> > > Set up a statically-allocated, dummy buffer to
> > > be used as flush buffer, which signals
> > > a encoding (or decoding) stop.
> > >
> > > When the flush buffer is queued to the OUTPUT queue,
> > > the driver will send an V4L2_EVENT_EOS event, and
> > > mark the CAPTURE buffer with V4L2_BUF_FLAG_LAST.
> >
> > I'm confused. What is the current driver doing wrong? It is already
> > setting the LAST flag AFAIK. I don't see why a dummy buffer is
> > needed.
>
> I'm not sure of this patch either. It seems to trigger the legacy
> "empty payload" buffer case. Driver should mark the last buffer, and
> then following poll should return EPIPE. Maybe it's the later that
> isn't respected ?
Sorry, I've send this too fast. The following poll should not block,
and DQBUF should retunr EPIPE.
In GStreamer we currently ignore the LAST flag and wait for EPIPE. The
reason is that not all driver can set the LAST flag. Exynos firmware
tells you it's done later and we don't want to introduce any latency in
the driver. The last flag isn't that useful in fact, but it can be use
with RTP to set the marker bit.
In previous discussion, using a buffer with payload 0 was not liked.
There might be codec where an empty buffer is valid, who knows ?
>
> >
> > Regards,
> >
> > Hans
> >
> > >
> > > With this change, it's possible to run a pipeline to completion:
> > >
> > > gst-launch-1.0 videotestsrc num-buffers=10 ! v4l2fwhtenc !
> > > v4l2fwhtdec ! fakevideosink
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> > > ---
> > > drivers/media/platform/vicodec/vicodec-core.c | 80 ++++++++++---
> > > --
> > > ----
> > > 1 file changed, 44 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/vicodec/vicodec-core.c
> > > b/drivers/media/platform/vicodec/vicodec-core.c
> > > index a2c487b4b80d..4ed4dae10e30 100644
> > > --- a/drivers/media/platform/vicodec/vicodec-core.c
> > > +++ b/drivers/media/platform/vicodec/vicodec-core.c
> > > @@ -113,7 +113,7 @@ struct vicodec_ctx {
> > > struct v4l2_ctrl_handler hdl;
> > >
> > > struct vb2_v4l2_buffer *last_src_buf;
> > > - struct vb2_v4l2_buffer *last_dst_buf;
> > > + struct vb2_v4l2_buffer flush_buf;
> > >
> > > /* Source and destination queue data */
> > > struct vicodec_q_data q_data[2];
> > > @@ -220,6 +220,7 @@ static void device_run(void *priv)
> > > struct vicodec_dev *dev = ctx->dev;
> > > struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > > struct vicodec_q_data *q_out;
> > > + bool flushing;
> > > u32 state;
> > >
> > > src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > > @@ -227,26 +228,36 @@ static void device_run(void *priv)
> > > q_out = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> > >
> > > state = VB2_BUF_STATE_DONE;
> > > - if (device_process(ctx, src_buf, dst_buf))
> > > +
> > > + flushing = (src_buf == &ctx->flush_buf);
> > > + if (!flushing && device_process(ctx, src_buf, dst_buf))
> > > state = VB2_BUF_STATE_ERROR;
> > > - ctx->last_dst_buf = dst_buf;
> > >
> > > spin_lock(ctx->lock);
> > > - if (!ctx->comp_has_next_frame && src_buf == ctx->last_src_buf)
> > > {
> > > + if (!flushing) {
> > > + if (!ctx->comp_has_next_frame && src_buf == ctx-
> > > > last_src_buf) {
> > >
> > > + dst_buf->flags |= V4L2_BUF_FLAG_LAST;
> > > + v4l2_event_queue_fh(&ctx->fh, &eos_event);
> > > + }
> > > +
> > > + if (ctx->is_enc) {
> > > + src_buf->sequence = q_out->sequence++;
> > > + src_buf = v4l2_m2m_src_buf_remove(ctx-
> > > > fh.m2m_ctx);
> > >
> > > + v4l2_m2m_buf_done(src_buf, state);
> > > + } else if (vb2_get_plane_payload(&src_buf->vb2_buf, 0)
> > > + == ctx->cur_buf_offset) {
> > > + src_buf->sequence = q_out->sequence++;
> > > + src_buf = v4l2_m2m_src_buf_remove(ctx-
> > > > fh.m2m_ctx);
> > >
> > > + v4l2_m2m_buf_done(src_buf, state);
> > > + ctx->cur_buf_offset = 0;
> > > + ctx->comp_has_next_frame = false;
> > > + }
> > > + } else {
> > > + src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > > + vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
> > > dst_buf->flags |= V4L2_BUF_FLAG_LAST;
> > > v4l2_event_queue_fh(&ctx->fh, &eos_event);
> > > }
> > > - if (ctx->is_enc) {
> > > - src_buf->sequence = q_out->sequence++;
> > > - src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > > - v4l2_m2m_buf_done(src_buf, state);
> > > - } else if (vb2_get_plane_payload(&src_buf->vb2_buf, 0) == ctx-
> > > > cur_buf_offset) {
> > >
> > > - src_buf->sequence = q_out->sequence++;
> > > - src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > > - v4l2_m2m_buf_done(src_buf, state);
> > > - ctx->cur_buf_offset = 0;
> > > - ctx->comp_has_next_frame = false;
> > > - }
> > > v4l2_m2m_buf_done(dst_buf, state);
> > > ctx->comp_size = 0;
> > > ctx->comp_magic_cnt = 0;
> > > @@ -293,6 +304,8 @@ static int job_ready(void *priv)
> > > src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > > if (!src_buf)
> > > return 0;
> > > + if (src_buf == &ctx->flush_buf)
> > > + return 1;
> > > p_out = vb2_plane_vaddr(&src_buf->vb2_buf, 0);
> > > sz = vb2_get_plane_payload(&src_buf->vb2_buf, 0);
> > > p = p_out + ctx->cur_buf_offset;
> > > @@ -770,21 +783,6 @@ static int vidioc_s_fmt_vid_out(struct file
> > > *file, void *priv,
> > > return ret;
> > > }
> > >
> > > -static void vicodec_mark_last_buf(struct vicodec_ctx *ctx)
> > > -{
> > > - static const struct v4l2_event eos_event = {
> > > - .type = V4L2_EVENT_EOS
> > > - };
> > > -
> > > - spin_lock(ctx->lock);
> > > - ctx->last_src_buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
> > > - if (!ctx->last_src_buf && ctx->last_dst_buf) {
> > > - ctx->last_dst_buf->flags |= V4L2_BUF_FLAG_LAST;
> > > - v4l2_event_queue_fh(&ctx->fh, &eos_event);
> > > - }
> > > - spin_unlock(ctx->lock);
> > > -}
> > > -
> > > static int vicodec_try_encoder_cmd(struct file *file, void *fh,
> > > struct v4l2_encoder_cmd *ec)
> > > {
> > > @@ -806,8 +804,8 @@ static int vicodec_encoder_cmd(struct file
> > > *file, void *fh,
> > > ret = vicodec_try_encoder_cmd(file, fh, ec);
> > > if (ret < 0)
> > > return ret;
> > > -
> > > - vicodec_mark_last_buf(ctx);
> > > + v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, &ctx->flush_buf);
> > > + v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
> > > return 0;
> > > }
> > >
> > > @@ -835,8 +833,8 @@ static int vicodec_decoder_cmd(struct file
> > > *file, void *fh,
> > > ret = vicodec_try_decoder_cmd(file, fh, dc);
> > > if (ret < 0)
> > > return ret;
> > > -
> > > - vicodec_mark_last_buf(ctx);
> > > + v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, &ctx->flush_buf);
> > > + v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
> > > return 0;
> > > }
> > >
> > > @@ -991,7 +989,7 @@ static void vicodec_return_bufs(struct
> > > vb2_queue *q, u32 state)
> > > vbuf = v4l2_m2m_src_buf_remove(ctx-
> > > > fh.m2m_ctx);
> > >
> > > else
> > > vbuf = v4l2_m2m_dst_buf_remove(ctx-
> > > > fh.m2m_ctx);
> > >
> > > - if (vbuf == NULL)
> > > + if (!vbuf || vbuf == &ctx->flush_buf)
> > > return;
> > > spin_lock(ctx->lock);
> > > v4l2_m2m_buf_done(vbuf, state);
> > > @@ -1031,7 +1029,6 @@ static int vicodec_start_streaming(struct
> > > vb2_queue *q,
> > > state->ref_frame.cb = state->ref_frame.luma + size;
> > > state->ref_frame.cr = state->ref_frame.cb + size / chroma_div;
> > > ctx->last_src_buf = NULL;
> > > - ctx->last_dst_buf = NULL;
> > > state->gop_cnt = 0;
> > > ctx->cur_buf_offset = 0;
> > > ctx->comp_size = 0;
> > > @@ -1158,6 +1155,7 @@ static int vicodec_open(struct file *file)
> > > struct vicodec_dev *dev = video_drvdata(file);
> > > struct vicodec_ctx *ctx = NULL;
> > > struct v4l2_ctrl_handler *hdl;
> > > + struct vb2_queue *vq;
> > > unsigned int size;
> > > int rc = 0;
> > >
> > > @@ -1234,6 +1232,16 @@ static int vicodec_open(struct file *file)
> > >
> > > v4l2_fh_add(&ctx->fh);
> > >
> > > + /* Setup a dummy flush buffer, used to signal
> > > + * encoding/decoding stop operation. When this buffer
> > > + * is queued to the OUTPUT queue, the driver will send
> > > + * V4L2_EVENT_EOS and send the last buffer to userspace.
> > > + */
> > > + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, multiplanar ?
> > > + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE :
> > > + V4L2_BUF_TYPE_VIDEO_OUTPUT);
> > > + ctx->flush_buf.vb2_buf.vb2_queue = vq;
> > > +
> > > open_unlock:
> > > mutex_unlock(vfd->lock);
> > > return rc;
> > >
> >
> >
Attachment:
signature.asc
Description: This is a digitally signed message part
