On 09/27/2017 09:31 AM, wm4 wrote:
On Wed, 27 Sep 2017 09:03:58 -0700
Jorge Ramirez-Ortiz <jorge.ramirez-or...@linaro.org> wrote:
On 09/27/2017 06:01 AM, wm4 wrote:
On Tue, 26 Sep 2017 16:22:23 -0700
Jorge Ramirez-Ortiz <jorge.ramirez-or...@linaro.org> wrote:
Stopping the codec when no more input is available causes captured
buffers that might be ready to be dequeued to be invalidated.
This commit follows the V4L2 API more closely:
1. on the last valid input buffer, it sets the V4L2_BUF_FLAG_LAST.
2. ffmpeg then will continue dequeuing captured buffers until EPIPE is
raised by the driver (EOF condition)
This also has the advantage of making the timeout on dequeuing capture
buffers while draining unnecessary.
---
libavcodec/v4l2_context.c | 162
++++++++++++++++++----------------------------
1 file changed, 64 insertions(+), 98 deletions(-)
I can't really comment on the logic of this code. So LGTM, just some
minor comments/questions.
- /* 0. handle errors */
+ /* handle errors */
(Apparently) unrelated cosmetic changes should usually be in separate
patches, but that's probably not worth the trouble in this case.
ACK. will do on any following patches - or I can do it on this one if you want.
Not necessary. In general we prefer separating unrelated changes, but I
think there's a limit to which we have to cause additional work to
follow this rule. (It's mainly in cases where the cosmetic changes are
significant or in completely different code.)
+ if (!frame) {
+ /* flag that we are draining */
+ ctx_to_m2mctx(ctx)->draining = 1;
+
+ /* send EOS */
+ avbuf->buf.m.planes[0].bytesused = 1;
+ avbuf->flags |= V4L2_BUF_FLAG_LAST;
/* handle the end of stream case */
+ } else {
+ ret = ff_v4l2_buffer_avframe_to_buf(frame, avbuf);
+ if (ret)
+ return ret;
+ }
Wouldn't it be better to switch the if/else bodies and avoid the
negation in the if condition?
yes
I am going to add a ff_v4l2_buffer_eos() to avoid exposing the bytesused at this
level as well.
will fix
That sounds good. So you're going to send a new patch? Then I'll hold
off applying this one.
yes, just testing.
it is much cleaner this way.
I looked [1] at how the V4L2_BUF_FLAG_LAST is used in the Venus driver and it
seems that I could either set bytesused to 0 _OR_ set the V4L2_BUF_FLAG_LAST
instead (the API suggests using the flag)
I will do both to make sure all cases are covered in drivers that might not use
the flag.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/qcom/venus/helpers.c?h=v4.14-rc2#n325
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
