Skip to content
This repository has been archived by the owner on Mar 22, 2022. It is now read-only.

Fix issues with frame ready callbacks #486

Merged
merged 5 commits into from
Jul 29, 2020
Merged

Conversation

fibann
Copy link
Member

@fibann fibann commented Jul 28, 2020

Fixes #151, a deadlock caused by subscription and frame-ready event trying to acquire the callback lock and the WebRTC worker thread at the same time, wrong behavior when using both I420A and ARGB32 callbacks, and a non-compiling test.

fibann added 3 commits July 28, 2020 19:13
Fixes cases where both I420A and ARGB32 callbacks are used.
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.
@fibann fibann requested a review from djee-ms July 28, 2020 18:16
Copy link
Member

@djee-ms djee-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments and suggestions, can directly merge if/when fixed.

fibann added 2 commits July 29, 2020 15:30
They can cause deadlocks and the only side effect of removing them is that a
handler may be called once (at most) after it has been removed. This is an
issue with any free-threaded C# event, so users should be aware of this and
handle any necessary synchronization.

Otherwise for consistency we should implement this pattern in all our
free-threaded events which would be impractical.
@fibann fibann merged commit 8d3dc07 into microsoft:master Jul 29, 2020
@fibann fibann deleted the fix-callbacks branch July 29, 2020 14:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redundant conversion to ARGB consumes a lot of CPU (HL2 cannot stream > 720p with hardware H.264)
2 participants