New issue
Advanced search Search tips

Issue 755887 link

Starred by 5 users
Status: Fixed
Owner:
akahuang@chromium.org
Closed: Today
Cc:
kcwu@chromium.org
posciak@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Handle "show_existing_frame" correctly in vp9_decoder.

Project Member Reported by owenlin@chromium.org, Aug 16 2017 
VDA should not directly output those frame before the client has returned the picture. 

However, we may not be able to wait for client to return the Picture. It could be a dead lock.

Please see https://b.corp.google.com/issues/64285344 for more details.

Comment 1 by owenlin@chromium.org, Feb 12 2018

Owner: akahuang@chromium.org

Comment 2 by akahuang@chromium.org, Jul 25 

To support it, VDA will send the same picture buffer directly without waiting for client to return it back. Therefore both VDA and client side need to count how many time VDA::Client::PictureReady() and VDA::ReusePictureBuffer() is called. When the number of calling VDA::ReusePictureBuffer() is same as VDA::Client::PictureReady(), then the picture buffer can be reused by VDA.

Comment 3 by benhenry@chromium.org, Aug 2

Status: Assigned (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 29 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e249e73fc18350dea393cecc8d344a5a355c1d6

commit 8e249e73fc18350dea393cecc8d344a5a355c1d6
Author: Chih-Yu Huang <akahuang@google.com>
Date: Sat Sep 29 01:46:16 2018

v4l2_SVDA, Vaapi_VDA: Support VP9 show_existing_frame feature.

For VP9 video, when the flag "show_existing_frame" is set, the decoder
should output the specified reference picture which is decoded
previously. Originally V4L2 slice VDA and Vaapi VDA didn't handle this
situation that the reference picture is sent to client but not
returned back.
This CL supports this feature at these two VDA. If the reference picture
is sent to client side, then VDA just sends it directly.

BUG= 755887 
TEST=./video_decode_accelerator_unittest --ozone-platform=gbm
     --test_video_data=vp90_2_10_show_existing_frame2.vp9.ivf:352:288:16:16:35:150:12
     on Kevin and Eve device.
TEST=Run AppRTC on Scarlet successfully
TEST=Pass media:media_unittests




Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I9876fa5aabaf15e9dff813ff5efd564d8991eee1
Reviewed-on: https://chromium-review.googlesource.com/1119726
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595286}
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/content/renderer/media/webrtc/rtc_video_decoder.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/content/renderer/media/webrtc/rtc_video_decoder.h
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/content/renderer/pepper/pepper_video_decoder_host.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/filters/gpu_video_decoder.h
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/ipc/service/picture_buffer_manager.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/ipc/service/picture_buffer_manager_unittest.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.h
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/vaapi/vaapi_common.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/vaapi/vaapi_common.h
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/vp9_decoder.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/vp9_picture.cc
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/gpu/vp9_picture.h
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/test/data/README
[add] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/test/data/vp90_2_10_show_existing_frame2.vp9.ivf
[add] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/test/data/vp90_2_10_show_existing_frame2.vp9.ivf.md5
[modify] https://crrev.com/8e249e73fc18350dea393cecc8d344a5a355c1d6/media/video/video_decode_accelerator.h
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 2 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e90cd145dd6709ae07f8ac43b29072c01a49d435

commit e90cd145dd6709ae07f8ac43b29072c01a49d435
Author: Dan Sanders <sandersd@chromium.org>
Date: Tue Oct 02 21:43:53 2018

Revert "v4l2_SVDA, Vaapi_VDA: Support VP9 show_existing_frame feature."

This reverts commit 8e249e73fc18350dea393cecc8d344a5a355c1d6.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=890629

Original change's description:
> v4l2_SVDA, Vaapi_VDA: Support VP9 show_existing_frame feature.
> 
> For VP9 video, when the flag "show_existing_frame" is set, the decoder
> should output the specified reference picture which is decoded
> previously. Originally V4L2 slice VDA and Vaapi VDA didn't handle this
> situation that the reference picture is sent to client but not
> returned back.
> This CL supports this feature at these two VDA. If the reference picture
> is sent to client side, then VDA just sends it directly.
> 
> BUG= 755887 
> TEST=./video_decode_accelerator_unittest --ozone-platform=gbm
>      --test_video_data=vp90_2_10_show_existing_frame2.vp9.ivf:352:288:16:16:35:150:12
>      on Kevin and Eve device.
> TEST=Run AppRTC on Scarlet successfully
> TEST=Pass media:media_unittests
> 
> 
> 
> 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: I9876fa5aabaf15e9dff813ff5efd564d8991eee1
> Reviewed-on: https://chromium-review.googlesource.com/1119726
> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
> Reviewed-by: Pawel Osciak <posciak@chromium.org>
> Reviewed-by: Bill Budge <bbudge@chromium.org>
> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595286}

TBR=posciak@chromium.org,bbudge@chromium.org,johnylin@chromium.org,haraken@chromium.org,sandersd@chromium.org,akahuang@chromium.org,emircan@chromium.org,hiroh@chromium.org,acourbot@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  755887 
Change-Id: I126684a435364cbf677092ad2c97b371fb105054
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/c/1257862
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595992}
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/content/renderer/media/webrtc/rtc_video_decoder.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/content/renderer/media/webrtc/rtc_video_decoder.h
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/content/renderer/pepper/pepper_video_decoder_host.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/filters/gpu_video_decoder.h
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/ipc/service/picture_buffer_manager.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/ipc/service/picture_buffer_manager_unittest.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.h
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/vaapi/vaapi_common.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/vaapi/vaapi_common.h
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/vp9_decoder.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/vp9_picture.cc
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/gpu/vp9_picture.h
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/test/data/README.md
[delete] https://crrev.com/8bd882a29b8ba297e2d2a13cd34c2612e81edaa4/media/test/data/vp90_2_10_show_existing_frame2.vp9.ivf
[delete] https://crrev.com/8bd882a29b8ba297e2d2a13cd34c2612e81edaa4/media/test/data/vp90_2_10_show_existing_frame2.vp9.ivf.md5
[modify] https://crrev.com/e90cd145dd6709ae07f8ac43b29072c01a49d435/media/video/video_decode_accelerator.h
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 23 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd5181ecb173efdef4163f066c27bd136fca01d5

commit cd5181ecb173efdef4163f066c27bd136fca01d5
Author: Chih-Yu Huang <akahuang@google.com>
Date: Tue Oct 23 06:33:06 2018

Reland "v4l2_SVDA, Vaapi_VDA: Support VP9 show_existing_frame feature.""

This reverts commit e90cd145dd6709ae07f8ac43b29072c01a49d435.

In gpu_video_decoder, |available_pictures_| tracks the number of
picture buffers which are not sent to display.
In DimissPictureBuffer(), we should not decrease the value if the
buffer also at display.

|available_pictures_| is only used at CanReadWithoutStalling() method,
and no one will call this method from GpuVideoDecoder, so we calculate
the value instead of tracking the value to reduce the code complexity.

BUG= 755887 
BUG=890629
BUG= 892514 
TEST=Play some videos at Youtube and check Chrome doesn't crash
TEST=pass video_YouTubeHTML5, video_ChromeVidResChangeHWDecode,
          video_VideoDecodeMemoryUsage, video_VideoSeek autotest

Original change's description:
> v4l2_SVDA, Vaapi_VDA: Support VP9 show_existing_frame feature.
>
> For VP9 video, when the flag "show_existing_frame" is set, the
decoder
> should output the specified reference picture which is decoded
> previously. Originally V4L2 slice VDA and Vaapi VDA didn't handle
this
> situation that the reference picture is sent to client but not
> returned back.
> This CL supports this feature at these two VDA. If the reference
picture
> is sent to client side, then VDA just sends it directly.
>
> BUG= 755887 
> TEST=./video_decode_accelerator_unittest --ozone-platform=gbm
>
--test_video_data=vp90_2_10_show_existing_frame2.vp9.ivf:352:288:16:16:35:150:12
>      on Kevin and Eve device.
> TEST=Run AppRTC on Scarlet successfully
> TEST=Pass media:media_unittests

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I3db1f5b650a5dec67bf46d439a6eeb4a2f3d995a
Reviewed-on: https://chromium-review.googlesource.com/c/1258787
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601861}
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/content/renderer/media/webrtc/rtc_video_decoder.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/content/renderer/media/webrtc/rtc_video_decoder.h
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/content/renderer/pepper/pepper_video_decoder_host.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/filters/gpu_video_decoder.h
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/ipc/service/picture_buffer_manager.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/ipc/service/picture_buffer_manager_unittest.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.h
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/vaapi/vaapi_common.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/vaapi/vaapi_common.h
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/vp9_decoder.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/vp9_picture.cc
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/gpu/vp9_picture.h
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/test/data/README.md
[add] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/test/data/vp90_2_10_show_existing_frame2.vp9.ivf
[add] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/test/data/vp90_2_10_show_existing_frame2.vp9.ivf.md5
[modify] https://crrev.com/cd5181ecb173efdef4163f066c27bd136fca01d5/media/video/video_decode_accelerator.h

Comment 7 by akahuang@chromium.org, Today (12 hours ago)

Status: Fixed (was: Assigned)
►

Sign in to add a comment