New issue
Advanced search Search tips

Issue 756436 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

WebRtcMediaStreamTrackAdapter getters should be callable on any thread

Project Member Reported by hbos@chromium.org, Aug 17 2017

Issue description

WebRtcMediaStreamTrackAdapter::web_track, webrtc_track and IsEqual should be callable from any thread and they should be const.

In order to make these methods accessible from any thread (and possibly make them const) the signaling of initialization must be redesigned in the event of remote tracks. Remote tracks are created on the signaling thread, initialization completes on the main thread. The main thread can call |EnsureTrackIsInitialized|, but the signaling thread would have to wait for a signal. Alternatively, enforce |is_initialized| and make |WebRtcMediaStreamTrackAdapterMap::GetRemoteTrackAdapter| wait for a signal as to always return initialized adapters.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 16

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

commit b1877f022d1b8b9532da1b26e288211345ff0451
Author: Henrik Boström <hbos@chromium.org>
Date: Mon Jul 16 09:39:04 2018

Remove WebRtcMediaStreamAdapterMap and WebRtcMediaStreamAdapter.

These classes were used to create and map blink-layer representations of
media streams with webrtc-layer representations of media streams.

The life-cycle of these streams were for some time complicated,
ownership was shared through reference counting between legacy stream
sets, senders and receivers. "GetOrCreate" was necessary in case the
stream already existed due to legacy APIs and threading restrictions
were different depending on if the stream was local or remote, with
destructor having to run on a different thread than it was created.
Because of the threading restrictions, ownership passing was made
explicit with "AdapterRef" classes, std::unique_ptr<>s and
ShallowCopy(). This is not very conventional or pretty.
The multi-thread usage required mutexes and this class has been
responsible for a couple of deadlock bugs and flaky tests due to
thread jumps in the destructor.

As of recently, stream objects only exist in the blink layer, and the
content/webrtc layer's notion of streams are just the stream IDs. CLs
listed prior in the referenced bugs have replaced function calls to
third_party/webrtc to use the "std::string stream_id"-versions.

With these classes no longer needed, we happily delete them.

For the time being, WebRtcMediaStreamTrackAdapter[Map] is still needed,
but this can likely be simplified during the Onion Souping.

TBR=pfeldman@chromium.org

Bug:  777617 , 756436
Change-Id: I629597181fcd4eca286887cf1ff1ebbb3f08c64e
Reviewed-on: https://chromium-review.googlesource.com/1131499
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575212}
[modify] https://crrev.com/b1877f022d1b8b9532da1b26e288211345ff0451/content/renderer/BUILD.gn
[modify] https://crrev.com/b1877f022d1b8b9532da1b26e288211345ff0451/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[delete] https://crrev.com/1de58a363448c4e44e33e316d08968a373201c3f/content/renderer/media/webrtc/webrtc_media_stream_adapter.cc
[delete] https://crrev.com/1de58a363448c4e44e33e316d08968a373201c3f/content/renderer/media/webrtc/webrtc_media_stream_adapter.h
[delete] https://crrev.com/1de58a363448c4e44e33e316d08968a373201c3f/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.cc
[delete] https://crrev.com/1de58a363448c4e44e33e316d08968a373201c3f/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.h
[delete] https://crrev.com/1de58a363448c4e44e33e316d08968a373201c3f/content/renderer/media/webrtc/webrtc_media_stream_adapter_map_unittest.cc
[delete] https://crrev.com/1de58a363448c4e44e33e316d08968a373201c3f/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc
[modify] https://crrev.com/b1877f022d1b8b9532da1b26e288211345ff0451/content/test/BUILD.gn

Sign in to add a comment