Skip to content

Commit dc37bda

Browse files
toniheicopybara-github
tonihei
authored and
copybara-github
committed
Add missing synchronization in ExperimentalBandwidthMeter
Issue: androidx/media#612 PiperOrigin-RevId: 563690229
1 parent 263671a commit dc37bda

File tree

4 files changed

+131
-21
lines changed

4 files changed

+131
-21
lines changed

‎library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java

+18-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import android.content.Context;
1919
import android.os.Handler;
20+
import androidx.annotation.GuardedBy;
2021
import androidx.annotation.Nullable;
2122
import com.google.android.exoplayer2.C;
2223
import com.google.android.exoplayer2.upstream.BandwidthMeter.EventListener.EventDispatcher;
@@ -286,20 +287,34 @@ public static synchronized DefaultBandwidthMeter getSingletonInstance(Context co
286287

287288
private final ImmutableMap<Integer, Long> initialBitrateEstimates;
288289
private final EventDispatcher eventDispatcher;
289-
private final SlidingPercentile slidingPercentile;
290290
private final Clock clock;
291291
private final boolean resetOnNetworkTypeChange;
292292

293+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
294+
private final SlidingPercentile slidingPercentile;
295+
296+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
293297
private int streamCount;
298+
299+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
294300
private long sampleStartTimeMs;
301+
302+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
295303
private long sampleBytesTransferred;
296304

297-
private @C.NetworkType int networkType;
305+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
298306
private long totalElapsedTimeMs;
307+
308+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
299309
private long totalBytesTransferred;
310+
311+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
300312
private long bitrateEstimate;
313+
314+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
301315
private long lastReportedBitrateEstimate;
302316

317+
private @C.NetworkType int networkType;
303318
private boolean networkTypeOverrideSet;
304319
private @C.NetworkType int networkTypeOverride;
305320

@@ -446,6 +461,7 @@ private synchronized void onNetworkTypeChanged(@C.NetworkType int networkType) {
446461
slidingPercentile.reset();
447462
}
448463

464+
@GuardedBy("this")
449465
private void maybeNotifyBandwidthSample(
450466
int elapsedMs, long bytesTransferred, long bitrateEstimate) {
451467
if (elapsedMs == 0 && bytesTransferred == 0 && bitrateEstimate == lastReportedBitrateEstimate) {

‎library/core/src/main/java/com/google/android/exoplayer2/upstream/experimental/ExperimentalBandwidthMeter.java

+11-5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import android.content.Context;
2121
import android.os.Handler;
22+
import androidx.annotation.GuardedBy;
2223
import androidx.annotation.Nullable;
2324
import com.google.android.exoplayer2.C;
2425
import com.google.android.exoplayer2.upstream.BandwidthMeter;
@@ -279,9 +280,13 @@ private static Map<Integer, Long> getInitialBitrateEstimatesForCountry(String co
279280
}
280281

281282
private final ImmutableMap<Integer, Long> initialBitrateEstimates;
283+
private final boolean resetOnNetworkTypeChange;
284+
285+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
282286
private final TimeToFirstByteEstimator timeToFirstByteEstimator;
287+
288+
@GuardedBy("this") // Used in TransferListener methods that are called on a background thread.
283289
private final BandwidthEstimator bandwidthEstimator;
284-
private final boolean resetOnNetworkTypeChange;
285290

286291
private @C.NetworkType int networkType;
287292
private long initialBitrateEstimate;
@@ -327,7 +332,7 @@ public synchronized long getBitrateEstimate() {
327332
}
328333

329334
@Override
330-
public long getTimeToFirstByteEstimateUs() {
335+
public synchronized long getTimeToFirstByteEstimateUs() {
331336
return timeToFirstByteEstimator.getTimeToFirstByteEstimateUs();
332337
}
333338

@@ -337,19 +342,20 @@ public TransferListener getTransferListener() {
337342
}
338343

339344
@Override
340-
public void addEventListener(Handler eventHandler, EventListener eventListener) {
345+
public synchronized void addEventListener(Handler eventHandler, EventListener eventListener) {
341346
checkNotNull(eventHandler);
342347
checkNotNull(eventListener);
343348
bandwidthEstimator.addEventListener(eventHandler, eventListener);
344349
}
345350

346351
@Override
347-
public void removeEventListener(EventListener eventListener) {
352+
public synchronized void removeEventListener(EventListener eventListener) {
348353
bandwidthEstimator.removeEventListener(eventListener);
349354
}
350355

351356
@Override
352-
public void onTransferInitializing(DataSource source, DataSpec dataSpec, boolean isNetwork) {
357+
public synchronized void onTransferInitializing(
358+
DataSource source, DataSpec dataSpec, boolean isNetwork) {
353359
if (!isTransferAtFullNetworkSpeed(dataSpec, isNetwork)) {
354360
return;
355361
}

‎library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeterTest.java

+39-7
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
@Config(sdk = Config.ALL_SDKS) // Test all SDKs because network detection logic changed over time.
5151
public final class DefaultBandwidthMeterTest {
5252

53-
private static final int SIMULATED_TRANSFER_COUNT = 100;
5453
private static final String FAST_COUNTRY_ISO = "TW";
5554
private static final String SLOW_COUNTRY_ISO = "PG";
5655

@@ -666,19 +665,21 @@ public void networkTypeOverride_doesFullReset() {
666665
new DefaultBandwidthMeter.Builder(ApplicationProvider.getApplicationContext())
667666
.setClock(clock)
668667
.build();
669-
long[] bitrateEstimatesWithNewInstance = simulateTransfers(bandwidthMeter, clock);
668+
long[] bitrateEstimatesWithNewInstance =
669+
simulateTransfers(bandwidthMeter, clock, /* simulatedTransferCount= */ 100);
670670

671671
// Create a new instance and seed with some transfers.
672672
setActiveNetworkInfo(networkInfo2g);
673673
bandwidthMeter =
674674
new DefaultBandwidthMeter.Builder(ApplicationProvider.getApplicationContext())
675675
.setClock(clock)
676676
.build();
677-
simulateTransfers(bandwidthMeter, clock);
677+
simulateTransfers(bandwidthMeter, clock, /* simulatedTransferCount= */ 100);
678678

679679
// Override the network type to ethernet and simulate transfers again.
680680
bandwidthMeter.setNetworkTypeOverride(C.NETWORK_TYPE_ETHERNET);
681-
long[] bitrateEstimatesAfterReset = simulateTransfers(bandwidthMeter, clock);
681+
long[] bitrateEstimatesAfterReset =
682+
simulateTransfers(bandwidthMeter, clock, /* simulatedTransferCount= */ 100);
682683

683684
// If overriding the network type fully reset the bandwidth meter, we expect the bitrate
684685
// estimates generated during simulation to be the same.
@@ -695,6 +696,36 @@ public void defaultInitialBitrateEstimate_withoutContext_isReasonable() {
695696
assertThat(initialEstimate).isLessThan(50_000_000L);
696697
}
697698

699+
@Test
700+
public void getBitrateEstimate_withSimultaneousTransferEvents_receivesUpdatedValues() {
701+
FakeClock clock = new FakeClock(/* initialTimeMs= */ 0);
702+
DefaultBandwidthMeter bandwidthMeter =
703+
new DefaultBandwidthMeter.Builder(ApplicationProvider.getApplicationContext())
704+
.setClock(clock)
705+
.build();
706+
707+
Thread thread =
708+
new Thread("backgroundTransfers") {
709+
@Override
710+
public void run() {
711+
simulateTransfers(bandwidthMeter, clock, /* simulatedTransferCount= */ 10000);
712+
}
713+
};
714+
thread.start();
715+
716+
long currentBitrateEstimate = bandwidthMeter.getBitrateEstimate();
717+
boolean bitrateEstimateUpdated = false;
718+
while (thread.isAlive()) {
719+
long newBitrateEstimate = bandwidthMeter.getBitrateEstimate();
720+
if (newBitrateEstimate != currentBitrateEstimate) {
721+
currentBitrateEstimate = newBitrateEstimate;
722+
bitrateEstimateUpdated = true;
723+
}
724+
}
725+
726+
assertThat(bitrateEstimateUpdated).isTrue();
727+
}
728+
698729
private void setActiveNetworkInfo(NetworkInfo networkInfo) {
699730
setActiveNetworkInfo(networkInfo, TelephonyDisplayInfo.OVERRIDE_NETWORK_TYPE_NONE);
700731
}
@@ -723,12 +754,13 @@ private void setNetworkCountryIso(String countryIso) {
723754
Shadows.shadowOf(telephonyManager).setNetworkCountryIso(countryIso);
724755
}
725756

726-
private static long[] simulateTransfers(DefaultBandwidthMeter bandwidthMeter, FakeClock clock) {
727-
long[] bitrateEstimates = new long[SIMULATED_TRANSFER_COUNT];
757+
private static long[] simulateTransfers(
758+
DefaultBandwidthMeter bandwidthMeter, FakeClock clock, int simulatedTransferCount) {
759+
long[] bitrateEstimates = new long[simulatedTransferCount];
728760
Random random = new Random(/* seed= */ 0);
729761
DataSource dataSource = new FakeDataSource();
730762
DataSpec dataSpec = new DataSpec(Uri.parse("https://test.com"));
731-
for (int i = 0; i < SIMULATED_TRANSFER_COUNT; i++) {
763+
for (int i = 0; i < simulatedTransferCount; i++) {
732764
bandwidthMeter.onTransferStart(dataSource, dataSpec, /* isNetwork= */ true);
733765
clock.advanceTime(random.nextInt(/* bound= */ 5000));
734766
bandwidthMeter.onBytesTransferred(

‎library/core/src/test/java/com/google/android/exoplayer2/upstream/experimental/ExperimentalBandwidthMeterTest.java

+63-7
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
@Config(sdk = Config.ALL_SDKS) // Test all SDKs because network detection logic changed over time.
5454
public final class ExperimentalBandwidthMeterTest {
5555

56-
private static final int SIMULATED_TRANSFER_COUNT = 100;
5756
private static final String FAST_COUNTRY_ISO = "TW";
5857
private static final String SLOW_COUNTRY_ISO = "PG";
5958

@@ -666,23 +665,79 @@ public void networkTypeOverride_doesFullReset() {
666665
setActiveNetworkInfo(networkInfoEthernet);
667666
ExperimentalBandwidthMeter bandwidthMeter =
668667
new ExperimentalBandwidthMeter.Builder(ApplicationProvider.getApplicationContext()).build();
669-
long[] bitrateEstimatesWithNewInstance = simulateTransfers(bandwidthMeter);
668+
long[] bitrateEstimatesWithNewInstance =
669+
simulateTransfers(bandwidthMeter, /* simulatedTransferCount= */ 100);
670670

671671
// Create a new instance and seed with some transfers.
672672
setActiveNetworkInfo(networkInfo2g);
673673
bandwidthMeter =
674674
new ExperimentalBandwidthMeter.Builder(ApplicationProvider.getApplicationContext()).build();
675-
simulateTransfers(bandwidthMeter);
675+
simulateTransfers(bandwidthMeter, /* simulatedTransferCount= */ 100);
676676

677677
// Override the network type to ethernet and simulate transfers again.
678678
bandwidthMeter.setNetworkTypeOverride(C.NETWORK_TYPE_ETHERNET);
679-
long[] bitrateEstimatesAfterReset = simulateTransfers(bandwidthMeter);
679+
long[] bitrateEstimatesAfterReset =
680+
simulateTransfers(bandwidthMeter, /* simulatedTransferCount= */ 100);
680681

681682
// If overriding the network type fully reset the bandwidth meter, we expect the bitrate
682683
// estimates generated during simulation to be the same.
683684
assertThat(bitrateEstimatesAfterReset).isEqualTo(bitrateEstimatesWithNewInstance);
684685
}
685686

687+
@Test
688+
public void getTimeToFirstByteEstimateUs_withSimultaneousTransferEvents_receivesUpdatedValues() {
689+
ExperimentalBandwidthMeter bandwidthMeter =
690+
new ExperimentalBandwidthMeter.Builder(ApplicationProvider.getApplicationContext()).build();
691+
692+
Thread thread =
693+
new Thread("backgroundTransfers") {
694+
@Override
695+
public void run() {
696+
simulateTransfers(bandwidthMeter, /* simulatedTransferCount= */ 10000);
697+
}
698+
};
699+
thread.start();
700+
701+
long currentTimeToFirstByteEstimateUs = bandwidthMeter.getTimeToFirstByteEstimateUs();
702+
boolean timeToFirstByteEstimateUpdated = false;
703+
while (thread.isAlive()) {
704+
long newTimeToFirstByteEstimateUs = bandwidthMeter.getTimeToFirstByteEstimateUs();
705+
if (newTimeToFirstByteEstimateUs != currentTimeToFirstByteEstimateUs) {
706+
currentTimeToFirstByteEstimateUs = newTimeToFirstByteEstimateUs;
707+
timeToFirstByteEstimateUpdated = true;
708+
}
709+
}
710+
711+
assertThat(timeToFirstByteEstimateUpdated).isTrue();
712+
}
713+
714+
@Test
715+
public void getBitrateEstimate_withSimultaneousTransferEvents_receivesUpdatedValues() {
716+
ExperimentalBandwidthMeter bandwidthMeter =
717+
new ExperimentalBandwidthMeter.Builder(ApplicationProvider.getApplicationContext()).build();
718+
719+
Thread thread =
720+
new Thread("backgroundTransfers") {
721+
@Override
722+
public void run() {
723+
simulateTransfers(bandwidthMeter, /* simulatedTransferCount= */ 10000);
724+
}
725+
};
726+
thread.start();
727+
728+
long currentBitrateEstimate = bandwidthMeter.getBitrateEstimate();
729+
boolean bitrateEstimateUpdated = false;
730+
while (thread.isAlive()) {
731+
long newBitrateEstimate = bandwidthMeter.getBitrateEstimate();
732+
if (newBitrateEstimate != currentBitrateEstimate) {
733+
currentBitrateEstimate = newBitrateEstimate;
734+
bitrateEstimateUpdated = true;
735+
}
736+
}
737+
738+
assertThat(bitrateEstimateUpdated).isTrue();
739+
}
740+
686741
private void setActiveNetworkInfo(NetworkInfo networkInfo) {
687742
setActiveNetworkInfo(networkInfo, TelephonyDisplayInfo.OVERRIDE_NETWORK_TYPE_NONE);
688743
}
@@ -711,12 +766,13 @@ private void setNetworkCountryIso(String countryIso) {
711766
Shadows.shadowOf(telephonyManager).setNetworkCountryIso(countryIso);
712767
}
713768

714-
private static long[] simulateTransfers(ExperimentalBandwidthMeter bandwidthMeter) {
715-
long[] bitrateEstimates = new long[SIMULATED_TRANSFER_COUNT];
769+
private static long[] simulateTransfers(
770+
ExperimentalBandwidthMeter bandwidthMeter, int simulatedTransferCount) {
771+
long[] bitrateEstimates = new long[simulatedTransferCount];
716772
Random random = new Random(/* seed= */ 0);
717773
DataSource dataSource = new FakeDataSource();
718774
DataSpec dataSpec = new DataSpec(Uri.parse("https://test.com"));
719-
for (int i = 0; i < SIMULATED_TRANSFER_COUNT; i++) {
775+
for (int i = 0; i < simulatedTransferCount; i++) {
720776
bandwidthMeter.onTransferInitializing(dataSource, dataSpec, /* isNetwork= */ true);
721777
ShadowSystemClock.advanceBy(Duration.ofMillis(random.nextInt(50)));
722778
bandwidthMeter.onTransferStart(dataSource, dataSpec, /* isNetwork= */ true);

0 commit comments

Comments
 (0)