-
Notifications
You must be signed in to change notification settings - Fork 283
Redundant conversion to ARGB consumes a lot of CPU (HL2 cannot stream > 720p with hardware H.264) #151
Comments
Further experiments show that just reverting that commit won't fix the issue on latest master. So some other cause was introduced between that commit and now. Will investigate more. |
Commit 1050e49 moved the offending callback to another file, so blindly reverting ab67d06 won't disable it. Cherry-pick fibann@2ae2408 instead. |
This has been fixed by #412 - video callbacks are now added only if there is at least one managed handler, so we don't waste cycles by default anymore. |
#412 implemented the correct pattern in video sources but did not backport it to video tracks, so this is still open. |
Registering a callback with the interop library triggers the conversion of every frame from the native format into the one the callback wants, which is very expensive and should be done only when needed. This also fixes a deadlock caused by subscription and frame-ready event trying to acquire the callback lock and the WebRTC worker thread at the same time. Fixes microsoft#151.
Registering a callback with the interop library triggers the conversion of every frame from the native format into the one the callback wants, which is very expensive and should be done only when needed. This also fixes a deadlock caused by subscription and frame-ready event trying to acquire the callback lock and the WebRTC worker thread at the same time. Fixes microsoft#151.
Registering a callback with the interop library triggers the conversion of every frame from the native format into the one the callback wants, which is very expensive and should be done only when needed. This also fixes a deadlock caused by subscription and frame-ready event trying to acquire the callback lock and the WebRTC worker thread at the same time. Fixes #151.
@fibann Can you clarify whether this issue affects encoding, decoding, or both? Your initial description here refers to starving the encoding thread, but this also references issue #160 which looks like a problem triggered by inbound video. I ask because we have an upcoming release of our product and are trying to decide whether to move to 2.0.0 for this release or wait until the next. The performance benefit of this fix would likely mean we would move to 2.0.0 immediately, but would prefer not to incur the risk (however minimal) if there is no immediate benefit. Thanks |
@kspark-scott this affects both local and remote video (note that it's not directly related to encoding, as long as you have a local track - even if you are not streaming it - you'll get the performance hit). While we recommend that you upgrade to 2.0 in general, you can simply cherry-pick fibann@2ae2408 (as said above) to work around this issue on 1.0.3 (note anyway that with that change the ARGB32 frame events will not be fired at all). |
Perfect, thanks. |
This is a regression starting from ab67d06 (so impacts releases starting from v1.0.1). It's not incredibly clear what's happening but the best guess is that adding the callbacks triggers the conversion of every frame to RGB, which starves the encoding thread.
This is going to be fixed as part of #152. If the RGB frames are not needed, a simple workaround is to revert the commit above.
The text was updated successfully, but these errors were encountered: