Repository navigation
fix: make track sinks safe to change from any thread and callback - #517
Merged
Merged
Conversation
AudioTrack and VideoTrack kept their sinks in unguarded maps, so two threads adding the same sink made two native registrations, and an add could overlap dispose(). A shared SinkRegistry now guards the map with a lock that is never held across a native call, and dispose() waits for the native calls still running.
A sink that removed itself in onVideoFrame() blocked for good: WebRTC holds the broadcaster's lock while it delivers a frame, and RemoveSink() takes it again on the worker thread the caller waits for. The same held for adding a sink, for audio sinks of a remote track or a CustomAudioSource, and for voice activity listeners. A change made within a callback now takes effect for the Java side at once and is made natively on another thread after the callback returned; dispose() there fails with an IllegalStateException.
VideoTrackSink copied every frame for each sink, so four sinks of a 1080p30 track copied about 356 MiB a second. addSink(sink, VideoFrameAccess.SHARED) hands a sink the pixels the source handed out, without a copy, in read-only byte buffers; frame.copy() gives a writable one. COPY stays the default and keeps sinks from seeing each other's writes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AudioTrackandVideoTrackare registered atomically: two threads adding the same sink make one native registration, anddispose()waits for native calls still running.dispose()there throws anIllegalStateException.addSink(sink, VideoFrameAccess.SHARED)hands a sink the source's pixels read-only instead of a copy per sink;COPYstays the default.