Migrate VEA over to new shared memory API |
||
Issue descriptionIn order to migrate the rest of the Video Capture stack over to the new shared memory API, content::VideoEncodeAccelerator::Encode() impls must be changed to account for any kind of memory backing of VideoFrames. For example, as of this writing, media::MojoVideoEncodeAccelerator DCHECKs that the legacy shmem API was used; and this inadvertently caused bug 888153 . That issue is fixed. However, this will become a problem for other use cases (e.g., most video capture cases, as covered by bug 843117).
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13981d5683f8b2fb6d0ea73521e72bf075b529b1 commit 13981d5683f8b2fb6d0ea73521e72bf075b529b1 Author: Yuri Wiitala <miu@chromium.org> Date: Thu Sep 27 21:35:52 2018 [CrOS+Cast] Fix shmem issue causing black screen share when Casting. Adds a memcpy (unfortunately) to allow content::VideoEncodeAccelerator to pass VideoFrame data to a HW encoder. Commit 4a74fb0fdb1fd882d4cc04cc had migrated the VIZ screen capturer to use the new read-only shmem API; but, unfortunately, the VEA framework for HW video encode still expects/requires the use of the legacy shmem API. Also added a little extra logic to more-gracefully halt encoding once a fatal error occurs. This issue was revealed while diagnosing the above problem, and is being fixed here since it is related. TBR=pthatcher@chromium.org Bug: 888153 , 888829 Change-Id: I7e37c024f4c8b2648769b98040e3d28597b199fc Reviewed-on: https://chromium-review.googlesource.com/1242396 Reviewed-by: Yuri Wiitala <miu@chromium.org> Commit-Queue: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/heads/master@{#594876} [modify] https://crrev.com/13981d5683f8b2fb6d0ea73521e72bf075b529b1/media/cast/sender/external_video_encoder.cc [modify] https://crrev.com/13981d5683f8b2fb6d0ea73521e72bf075b529b1/media/cast/sender/external_video_encoder.h [modify] https://crrev.com/13981d5683f8b2fb6d0ea73521e72bf075b529b1/media/cast/sender/video_encoder_unittest.cc [modify] https://crrev.com/13981d5683f8b2fb6d0ea73521e72bf075b529b1/media/cast/sender/video_sender_unittest.cc
,
Sep 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3fff72e0471322cb300a4c5b6865ea6215402ad3 commit 3fff72e0471322cb300a4c5b6865ea6215402ad3 Author: Yuri Wiitala <miu@chromium.org> Date: Fri Sep 28 21:57:17 2018 [CrOS+Cast] Fix shmem issue causing black screen share when Casting. Adds a memcpy (unfortunately) to allow content::VideoEncodeAccelerator to pass VideoFrame data to a HW encoder. Commit 4a74fb0fdb1fd882d4cc04cc had migrated the VIZ screen capturer to use the new read-only shmem API; but, unfortunately, the VEA framework for HW video encode still expects/requires the use of the legacy shmem API. Also added a little extra logic to more-gracefully halt encoding once a fatal error occurs. This issue was revealed while diagnosing the above problem, and is being fixed here since it is related. TBR=pthatcher@chromium.org Bug: 888153 , 888829 Change-Id: I7e37c024f4c8b2648769b98040e3d28597b199fc Reviewed-on: https://chromium-review.googlesource.com/1242396 Reviewed-by: Yuri Wiitala <miu@chromium.org> Commit-Queue: Yuri Wiitala <miu@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#594876}(cherry picked from commit 13981d5683f8b2fb6d0ea73521e72bf075b529b1) Reviewed-on: https://chromium-review.googlesource.com/1252847 Cr-Commit-Position: refs/branch-heads/3538@{#755} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/3fff72e0471322cb300a4c5b6865ea6215402ad3/media/cast/sender/external_video_encoder.cc [modify] https://crrev.com/3fff72e0471322cb300a4c5b6865ea6215402ad3/media/cast/sender/external_video_encoder.h [modify] https://crrev.com/3fff72e0471322cb300a4c5b6865ea6215402ad3/media/cast/sender/video_encoder_unittest.cc [modify] https://crrev.com/3fff72e0471322cb300a4c5b6865ea6215402ad3/media/cast/sender/video_sender_unittest.cc
,
Oct 3
Looks like we just added one more to the already existing memory copies/format conversions in the pipeline. I guess it only affects the Chrome capture stack, so wouldn't affect any CTS performance benchmarks for ARC++ use cases fortunately. Still, I expect this is a performance/power regression for Chrome use cases (Hangouts/Cast) affecting users, especially on lower-end devices. Did we have any performance comparison done before landing this change? |
||
►
Sign in to add a comment |
||
Comment 1 by posciak@chromium.org
, Sep 27