Android: Fix VideoTrack behavior for adding/removing VideoSinks
In particular:
* Trying to remove a VideoSink that was not attached should be a no-op.
* Trying to remove a null VideoSink should be a no-op.
* Adding the same VideoSink multiple times should a no-op, and should
not create duplicate native VideoSinks.
* Trying to add a null VideoSink should throw an exception in the Java
layer instead of crashing in native code.
This CL also adds tests that verify these behaviors.
Bug: webrtc:9403
Change-Id: I928b7bb7f683634e287d7fec9e26f4179f73c150
Reviewed-on: https://webrtc-review.googlesource.com/83322
Commit-Queue: Magnus Jedvert <magjed@webrtc.org>
Reviewed-by: Paulina Hensman <phensman@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23602}diff --git a/sdk/android/api/org/webrtc/VideoTrack.java b/sdk/android/api/org/webrtc/VideoTrack.java
index 214078b..a5a6eb0 100644
--- a/sdk/android/api/org/webrtc/VideoTrack.java
+++ b/sdk/android/api/org/webrtc/VideoTrack.java
@@ -32,9 +32,16 @@
* sources produce VideoFrames.
*/
public void addSink(VideoSink sink) {
- final long nativeSink = nativeWrapSink(sink);
- sinks.put(sink, nativeSink);
- nativeAddSink(nativeTrack, nativeSink);
+ if (sink == null) {
+ throw new IllegalArgumentException("The VideoSink is not allowed to be null");
+ }
+ // We allow calling addSink() with the same sink multiple times. This is similar to the C++
+ // VideoTrack::AddOrUpdateSink().
+ if (!sinks.containsKey(sink)) {
+ final long nativeSink = nativeWrapSink(sink);
+ sinks.put(sink, nativeSink);
+ nativeAddSink(nativeTrack, nativeSink);
+ }
}
/**
@@ -43,8 +50,8 @@
* If the VideoSink was not attached to the track, this is a no-op.
*/
public void removeSink(VideoSink sink) {
- final long nativeSink = sinks.remove(sink);
- if (nativeSink != 0) {
+ final Long nativeSink = sinks.remove(sink);
+ if (nativeSink != null) {
nativeRemoveSink(nativeTrack, nativeSink);
nativeFreeSink(nativeSink);
}