Make AVDACodecAllocator safe to use from multiple threads |
||
Issue descriptionMCVD and AVDA need to both be able to use this concurrently (AVDA from GPU thread for WebRTC, MCVD from media service thread).
,
Sep 15 2017
Thanks! Let me do a quick brain dump of related things: * MCVD doesn't support CVV. So no sync SurfaceDestroyed stuff to worry about. * I'm thinking we will just use AndroidSurfaceChooser and AndroidOverlayMojos only on the gpu main thread. Without sync surfacedestroyed it should be fine to host these on a different thread. * Thinking that making AVDASurfaceBundle RefCountedDeleteOnSequence would be a good idea to handle some edge cases with codec creation when MCVD goes away (SurfaceTextureGLOwner is an example).
,
Sep 15 2017
+1 RedCountedDeleteOnSequence. some thoughts on AVDASurfaceChooser: besides destroy, there's very little that we do with AndroidOverlayMojo in AVDA: get its java surface, and register a destruction callback is the whole list, i think. i think it'll be the same in MCVD, and the last two can just be folded into surfacechooser to avoid multi-threading in AO entirely, i think. as in, surfacechooser can be told during Initialize() what DestroyedCB the client would like to use. it can then set the destruction callback on its own thread before notifying the client when the overlay is ready. MCVD would provide a thread-hopping callback. actually, it has no choice but to do it before providing the overlay, if UseOverlayCB hops threads to MCVD thread. else the overlay could be destroyed before the MCVD thread registers the destruction callback. for GetJavaSurface, we can just send it as an additional argument to UseOverlayCB. not sure what the rules with JavaRef and threads are, but pretty sure there's a way to make it work. this is somewhat hacky, i suppose.
,
Sep 15 2017
i think that i have a plan for AVDACodecAllocator. the use of WeakPtrs to ensure that threads are stopped correctly makes it harder. i think i'm going to make all the calls post to the gpu main thread if they're called elsewhere. this enforces proper ordering. that's the only way i could see to ensure that stopping the codec threads can be done without a risk of something else posting to them. will also make StartThread return success asynchronously.
,
Oct 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e commit 04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e Author: liberato@chromium.org <liberato@chromium.org> Date: Wed Oct 04 18:46:28 2017 Make AVDACodecAllocator thread safe. AVDACodecAllocator previously was designed to work with AVDA on the GPU main thread. This CL makes it work properly even if clients call it from other threads, such as MCVD will do. It does three main things: - Create/ReleaseCodec calls post to the thread on which the allocator was constructed. This keeps everything ordered. Otherwise, it's very difficult to ensure that one client stopping threads won't happen concurrently with another posting tasks. Locks worked, but this prevents blocking the caller, and makes it easier to deal with the weakptr used to cancel StopThreadTask. - Responses to the client are on whatever thread it called on. This lets us continue to use client-provided WeakPtrs. - StartThread now sends its results asynchronously, since it posts to another thread sometimes. Bug: 760822 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ib1bca55602a2f43ea9e2ecfd55fa98c877fb6217 Reviewed-on: https://chromium-review.googlesource.com/669799 Commit-Queue: Frank Liberato <liberato@chromium.org> Reviewed-by: Chris Watkins <watk@chromium.org> Cr-Commit-Position: refs/heads/master@{#506463} [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/base/android/BUILD.gn [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/base/android/media_codec_bridge.h [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/base/android/media_codec_bridge_impl.h [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/base/android/mock_media_codec_bridge.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/base/android/mock_media_codec_bridge.h [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/android_video_decode_accelerator.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/android_video_decode_accelerator_unittest.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/avda_codec_allocator.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/avda_codec_allocator.h [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/avda_codec_allocator_unittest.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/content_video_view_overlay_allocator.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/content_video_view_overlay_allocator.h [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/content_video_view_overlay_allocator_unittest.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/fake_codec_allocator.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/fake_codec_allocator.h [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/media_codec_video_decoder.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/android/media_codec_video_decoder_unittest.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/gpu/gpu_video_decode_accelerator_factory.cc [modify] https://crrev.com/04ed2188b440cc9d7fc6b4fa3808368ea7f5ab0e/media/mojo/services/gpu_mojo_media_client.cc
,
Oct 4 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by liber...@chromium.org
, Sep 15 2017Owner: liber...@chromium.org