Hi Andriy, thanks for you patch. Le samedi 02 mai 2020 à 15:40 -0400, Andriy Gelman a écrit : > From: Andriy Gelman <andriy.gelman@xxxxxxxxx> > > As per V4L2 api, the final buffer should set V4L2_BUF_FLAG_LAST flag. > > Signed-off-by: Andriy Gelman <andriy.gelman@xxxxxxxxx> > --- > drivers/media/platform/s5p-mfc/s5p_mfc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c > index 5c2a23b953a4..b3d9b3a523fe 100644 > --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c > @@ -614,6 +614,7 @@ static void s5p_mfc_handle_stream_complete(struct s5p_mfc_ctx *ctx) > list_del(&mb_entry->list); > ctx->dst_queue_cnt--; > vb2_set_plane_payload(&mb_entry->b->vb2_buf, 0, 0); > + mb_entry->b->flags |= V4L2_BUF_FLAG_LAST; > vb2_buffer_done(&mb_entry->b->vb2_buf, VB2_BUF_STATE_DONE); The empty buffer is only there for backward compatibility. As the spec says, userspace should completely ignore this buffer. I bet it will still have the effect to set last_buffer_dequeued = true in vb2, unblocking poll() operations and allowing for the queue to unblock and return EPIPE on further DQBUF. Perhaps you should make sure the if both the src and dst queues are empty/done by the time cmd_stop is called it will still work. Other drivers seems to handle this, but this one does not seems to have a special case for that, which may hang userspace in a different way. What you should do to verify this patch is correct, and that your userpace does not rely on legacy path is that it should always be able to detect the end of the drain with a EPIPE on DQBUF. LAST_BUF is just an early signalling, but may not occur if there was nothing left to produce (except for MFC which maybe be crafting a buffer in all cases, but that's going a roundtrip through the HW, which is not clear will work if the dst queue was empty). As shared on IRC, you have sent these patch to FFMPEG: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200429212942.28797-2-andriy.gelman@xxxxxxxxx/ This should have been clarified as supporting legacy drivers / older kernel with Samsung driver. Seems like a fair patch. And you added: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200429212942.28797-1-andriy.gelman@xxxxxxxxx/ This one should maybe add the clarification that this is an optimization to skip an extra poll/dqbuf dance, but that end of draining will ultimately be catched by EPIPE on dqbuf for the described cases. Remains valid enhancement to ffmpeg imho. > } >
