New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 652923 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue 542522



Sign in to add a comment

Avoid using main render thread in WebRTC render path

Project Member Reported by emir...@chromium.org, Oct 5 2016

Issue description

We came across to this issue when debugging https://buganizer.corp.google.com/issues/31390397#comment84. Regular video playback(Youtube) and audio playback continues when main render thread is busy with JS functions. However, WebRTC video playback stalls and starts accumulating frames resulting in increased memory. WebRTC has been posting frames from IO thread[0] to render main thread[1] where MediaStreamVideoRendererSink and WebMediaPlayerMS. We can use a different thread for this path.

[0] https://cs.chromium.org/chromium/src/content/renderer/media/video_track_adapter.cc?rcl=1475162463&l=307
[1] https://cs.chromium.org/chromium/src/content/renderer/media/media_stream_video_renderer_sink.cc?rcl=14when75162463&l=54

 
Cc: dcasta...@chromium.org

Comment 2 by guidou@chromium.org, Oct 12 2016

Cc: magjed@chromium.org
Owner: emir...@chromium.org
Status: Started (was: Untriaged)
We saw the below issue on the bots, where lots frames are accumulated on vp9_frame_buffer_pool and we hit a check. I am attaching the logs here as it might be related to this.
https://build.chromium.org/p/chromium.webrtc/builders/Win10%20Tester/builds/11699/steps/browser_tests/logs/stdio
https://paste.googleplex.com/6101213393190912

Comment 4 by perkj@chromium.org, Nov 2 2016

Cc: perkj@chromium.org
Please also see https://bugs.chromium.org/p/chromium/issues/detail?id=542522
Blocking: 542522
See also https://bugs.chromium.org/p/webrtc/issues/detail?id=6484&can=2&start=0&num=100&q=&colspec=ID%20Pri%20M%20ReleaseBlock%20Component%20Status%20Owner%20Summary&groupby=&sort=: The tests noticed these VP9 pool crashes about a month ago (was flaky and not consistent though):

[3144:4708:1005/062402:WARNING:vp9_frame_buffer_pool.cc(72)] 69 Vp9FrameBuffers have been allocated by a Vp9FrameBufferPool (exceeding what is considered reasonable, 68).
#
# Fatal error in e:\b\c\b\win_builder\src\third_party\webrtc\modules\video_coding\codecs\vp9\vp9_frame_buffer_pool.cc, line 76
# last system error: 0
# Check failed: false
# 
#

Comment 7 by perkj@chromium.org, Nov 4 2016

Related to 6# - These test use the MediaStreamRecorder API right? No heavy java script.
Labels: -M-56 M-57
Bumping to M57. Please update if that's wrong.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 19 2016

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

commit 99eb6bc119173a68cc695f5179c72cadf0b515da
Author: emircan <emircan@chromium.org>
Date: Sat Nov 19 00:49:21 2016

Move passing of WebRTC rendering frames from main thread to compositor thread

When main render thread is busy with JS functions, WebRTC video playback stalls
and starts accumulating frames resulting in increased memory. It sometimes
recovers by playing the accumulated frames quickly, and sometimes causes out
of memory error.

This CL modifies the passing to video frames in rendering path such that main
thread is not used. We jump from IO thread directly to compositor thread and
avoid trampolining in between. Since lifetime and methods of these classes, such
as WebMediaPlayerMS, is still tied to main thread, I created inner classes which
are pinned to specific threads and carry forwarding tasks:
- FrameReceiver receives frames on IO and forwards to compositor.
- FrameDeliverer forwards frames on compositor.

BUG= 652923 
TEST=Modified unit tests. Tested running JS alert() in console during an AppRTC
loopback.
I am still confused to how/where to add a proper browsertest. Running alert() on
a page requires manual user interaction.

Review-Url: https://codereview.chromium.org/2472273002
Cr-Commit-Position: refs/heads/master@{#433356}

[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/public/renderer/media_stream_renderer_factory.h
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/public/renderer/media_stream_video_renderer.h
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/media_stream_renderer_factory_impl.cc
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/media_stream_renderer_factory_impl.h
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/media_stream_video_renderer_sink.cc
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/media_stream_video_renderer_sink.h
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/media_stream_video_renderer_sink_unittest.cc
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/webmediaplayer_ms.h
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/webmediaplayer_ms_compositor.h
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/shell/renderer/layout_test/test_media_stream_renderer_factory.cc
[modify] https://crrev.com/99eb6bc119173a68cc695f5179c72cadf0b515da/content/shell/renderer/layout_test/test_media_stream_renderer_factory.h

Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 9 2016

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

commit 8f71ad428b65fc259b48c2a0917158a762a5e5f6
Author: emircan <emircan@chromium.org>
Date: Fri Dec 09 18:44:21 2016

Move passing of WebRTC rendering frames to IO thread

This is follow-up for CL[0] that moved passing of WebRTC rendering frames
off of main thread. That CL caused a regression in test[1] where the number
of unique frames encoded is less, suggesting that we are dropping frames.
Traces showed that compositor thread may be blocked by long texture upload
calls and frames get accumulated in the meantime. We might be dropping
frames due to the limit of having only 3 frames in flight.

This CL addresses this issue by moving these calls to IO thread directly.
In order to do it:
- FrameDeliverer classes in MediaStreamVideoRendererSink and WebMediaPlayerMS
are moved to IO thread.
- WebMediaPlayerMSCompositor::EnqueueFrame gets called on IO thread.
- Since WebMediaPlayerMSCompositor operates on three threads, I made it a ref
counted object. This way, we do not need to create multiple WeakPtr factories
or synchronize/block destruction on multiple threads.

[0] https://codereview.chromium.org/2472273002/
[1]  crbug.com/667262 

BUG= 667262 , 652923 
TEST=Tested running JS alert() in console during an AppRTC loopback.

Review-Url: https://codereview.chromium.org/2529263004
Cr-Commit-Position: refs/heads/master@{#437594}

[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/public/renderer/media_stream_renderer_factory.h
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/media_stream_renderer_factory_impl.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/media_stream_renderer_factory_impl.h
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/media_stream_video_renderer_sink.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/media_stream_video_renderer_sink.h
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/media_stream_video_renderer_sink_unittest.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/webmediaplayer_ms.h
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/webmediaplayer_ms_compositor.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/webmediaplayer_ms_compositor.h
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/shell/renderer/layout_test/test_media_stream_renderer_factory.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/shell/renderer/layout_test/test_media_stream_renderer_factory.h
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/shell/renderer/layout_test/test_media_stream_video_renderer.cc
[modify] https://crrev.com/8f71ad428b65fc259b48c2a0917158a762a5e5f6/content/shell/renderer/layout_test/test_media_stream_video_renderer.h

Sign in to add a comment