Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2074133002:
Always enqueue an empty buffer when flush (Closed)
|
Created:
4 years, 6 months ago by henryhsu Modified:
4 years, 6 months ago CC:
chromium-reviews, posciak+watch_chromium.org, piman+watch_chromium.org, feature-media-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAlways enqueue an empty buffer when flush
If decoder receives flush after surface ran out, decoder does not
flush the last frame since decoder is waiting the surface.
Enqueue an empty buffer and it will trigger flush after all pending
inputs are done.
BUG=b:29403962
TEST=cts test pass, vda unittest pass, crosvideo.appspot.com works well
Committed: https://crrev.com/25d324e3026a26190f7eacc5810a5f612b814ba1
Cr-Commit-Position: refs/heads/master@{#400626}
Patch Set 1 #
Total comments: 7
Patch Set 2 : address review comments #Messages
Total messages: 16 (6 generated)
henryhsu@chromium.org changed reviewers: + owenlin@chromium.org, posciak@chromium.org
kcwu@chromium.org changed reviewers: + kcwu@chromium.org
https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... File media/gpu/v4l2_slice_video_decode_accelerator.cc (right): https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... media/gpu/v4l2_slice_video_decode_accelerator.cc:1846: return; delete this line
https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... File media/gpu/v4l2_slice_video_decode_accelerator.cc (right): https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... media/gpu/v4l2_slice_video_decode_accelerator.cc:1846: return; Add one line in the end: ScheduleDecodeBufferTaskIfNeeded()
https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... File media/gpu/v4l2_slice_video_decode_accelerator.cc (right): https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... media/gpu/v4l2_slice_video_decode_accelerator.cc:1841: // We are not done with pending inputs, so queue an empty buffer, Please also update the comment, how about: Queue an empty buffer will trigger flush sequence after we are done with all pending inputs.
Please test this change with vdatest and crosvideo.appspot.com. https://chromiumcodereview.appspot.com/2074133002/diff/1/media/gpu/v4l2_slice... File media/gpu/v4l2_slice_video_decode_accelerator.cc (right): https://chromiumcodereview.appspot.com/2074133002/diff/1/media/gpu/v4l2_slice... media/gpu/v4l2_slice_video_decode_accelerator.cc:1841: // We are not done with pending inputs, so queue an empty buffer, We actually may be done with pending inputs here already. Since the if() is being removed, instead of checking if inputs are pending and queuing an empty buffer only if they are, we unconditionally queue one. How about: "Queue an empty buffer which - when reached - will trigger the flush sequence."
tested with vdatest and crosvideo.appspot.com. https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... File media/gpu/v4l2_slice_video_decode_accelerator.cc (right): https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... media/gpu/v4l2_slice_video_decode_accelerator.cc:1841: // We are not done with pending inputs, so queue an empty buffer, On 2016/06/20 00:55:53, Pawel Osciak wrote: > We actually may be done with pending inputs here already. Since the if() is > being removed, instead of checking if inputs are pending and queuing an empty > buffer only if they are, we unconditionally queue one. > > How about: > > "Queue an empty buffer which - when reached - will trigger the flush sequence." Done. https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... media/gpu/v4l2_slice_video_decode_accelerator.cc:1846: return; On 2016/06/17 12:17:49, kcwu wrote: > delete this line Done. https://codereview.chromium.org/2074133002/diff/1/media/gpu/v4l2_slice_video_... media/gpu/v4l2_slice_video_decode_accelerator.cc:1846: return; On 2016/06/17 13:35:42, Owen Lin wrote: > Add one line in the end: > ScheduleDecodeBufferTaskIfNeeded() Done.
lgtm, but please update the TEST= line in CL message.
Description was changed from ========== Always enqueue an empty buffer when flush If decoder receives flush after surface ran out, decoder does not flush the last frame since decoder is waiting the surface. Enqueue an empty buffer and it will trigger flush after all pending inputs are done. BUG=b:29403962 TEST=cts test pass ========== to ========== Always enqueue an empty buffer when flush If decoder receives flush after surface ran out, decoder does not flush the last frame since decoder is waiting the surface. Enqueue an empty buffer and it will trigger flush after all pending inputs are done. BUG=b:29403962 TEST=cts test pass, vda unittest pass, crosvideo.appspot.com works well ==========
The CQ bit was checked by henryhsu@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2074133002/20001
Message was sent while issue was closed.
Description was changed from ========== Always enqueue an empty buffer when flush If decoder receives flush after surface ran out, decoder does not flush the last frame since decoder is waiting the surface. Enqueue an empty buffer and it will trigger flush after all pending inputs are done. BUG=b:29403962 TEST=cts test pass, vda unittest pass, crosvideo.appspot.com works well ========== to ========== Always enqueue an empty buffer when flush If decoder receives flush after surface ran out, decoder does not flush the last frame since decoder is waiting the surface. Enqueue an empty buffer and it will trigger flush after all pending inputs are done. BUG=b:29403962 TEST=cts test pass, vda unittest pass, crosvideo.appspot.com works well ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Always enqueue an empty buffer when flush If decoder receives flush after surface ran out, decoder does not flush the last frame since decoder is waiting the surface. Enqueue an empty buffer and it will trigger flush after all pending inputs are done. BUG=b:29403962 TEST=cts test pass, vda unittest pass, crosvideo.appspot.com works well ========== to ========== Always enqueue an empty buffer when flush If decoder receives flush after surface ran out, decoder does not flush the last frame since decoder is waiting the surface. Enqueue an empty buffer and it will trigger flush after all pending inputs are done. BUG=b:29403962 TEST=cts test pass, vda unittest pass, crosvideo.appspot.com works well Committed: https://crrev.com/25d324e3026a26190f7eacc5810a5f612b814ba1 Cr-Commit-Position: refs/heads/master@{#400626} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/25d324e3026a26190f7eacc5810a5f612b814ba1 Cr-Commit-Position: refs/heads/master@{#400626} |
