From 5f6c907c652c70f44869a7a7e7f70573fc88d89c Mon Sep 17 00:00:00 2001 From: olly Date: Wed, 31 Oct 2018 04:58:25 -0700 Subject: [PATCH] Reset DefaultBandwidthMeter on network type change ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=219454931 --- .../upstream/DefaultBandwidthMeter.java | 171 ++++++++++++++++-- .../exoplayer2/util/SlidingPercentile.java | 8 + .../google/android/exoplayer2/util/Util.java | 2 +- .../upstream/DefaultBandwidthMeterTest.java | 74 ++++++-- 4 files changed, 225 insertions(+), 30 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java index 6be13603ee..eef86a3aab 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java @@ -15,8 +15,13 @@ */ package com.google.android.exoplayer2.upstream; +import android.content.BroadcastReceiver; import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; +import android.net.ConnectivityManager; import android.os.Handler; +import android.os.Looper; import android.support.annotation.Nullable; import android.util.SparseArray; import com.google.android.exoplayer2.C; @@ -25,9 +30,12 @@ import com.google.android.exoplayer2.util.Clock; import com.google.android.exoplayer2.util.EventDispatcher; import com.google.android.exoplayer2.util.SlidingPercentile; import com.google.android.exoplayer2.util.Util; +import java.lang.ref.WeakReference; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** * Estimates bandwidth by listening to data transfers. @@ -79,6 +87,7 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList private SparseArray initialBitrateEstimates; private int slidingWindowMaxWeight; private Clock clock; + private boolean resetOnNetworkTypeChange; /** * Creates a builder with default parameters and without listener. @@ -119,9 +128,8 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList } /** - * Sets the initial bitrate estimate in bits per second for a network type that should be - * assumed when a bandwidth estimate is unavailable and the current network connection is of the - * specified type. + * Sets the initial bitrate estimate in bits per second that should be assumed when a bandwidth + * estimate is unavailable and the current network connection is of the specified type. * * @param networkType The {@link C.NetworkType} this initial estimate is for. * @param initialBitrateEstimate The initial bitrate estimate in bits per second. @@ -159,16 +167,29 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList return this; } + /** + * Sets whether to reset if the network type changes. + * + * @param resetOnNetworkTypeChange Whether to reset if the network type changes. + * @return This builder. + */ + public Builder experimental_resetOnNetworkTypeChange(boolean resetOnNetworkTypeChange) { + this.resetOnNetworkTypeChange = resetOnNetworkTypeChange; + return this; + } + /** * Builds the bandwidth meter. * * @return A bandwidth meter with the configured properties. */ public DefaultBandwidthMeter build() { - DefaultBandwidthMeter bandwidthMeter = - new DefaultBandwidthMeter( - context, initialBitrateEstimates, slidingWindowMaxWeight, clock); - return bandwidthMeter; + return new DefaultBandwidthMeter( + context, + initialBitrateEstimates, + slidingWindowMaxWeight, + clock, + resetOnNetworkTypeChange); } private static SparseArray getInitialBitrateEstimatesForCountry(String countryCode) { @@ -195,6 +216,7 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList private static final int ELAPSED_MILLIS_FOR_ESTIMATE = 2000; private static final int BYTES_TRANSFERRED_FOR_ESTIMATE = 512 * 1024; + @Nullable private final Context context; private final SparseArray initialBitrateEstimates; private final EventDispatcher eventDispatcher; private final SlidingPercentile slidingPercentile; @@ -204,9 +226,14 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList private long sampleStartTimeMs; private long sampleBytesTransferred; + @C.NetworkType private int networkType; private long totalElapsedTimeMs; private long totalBytesTransferred; private long bitrateEstimate; + private long lastReportedBitrateEstimate; + + private boolean networkTypeOverrideSet; + @C.NetworkType private int networkTypeOverride; /** @deprecated Use {@link Builder} instead. */ @Deprecated @@ -215,34 +242,44 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList /* context= */ null, /* initialBitrateEstimates= */ new SparseArray<>(), DEFAULT_SLIDING_WINDOW_MAX_WEIGHT, - Clock.DEFAULT); + Clock.DEFAULT, + /* resetOnNetworkTypeChange= */ false); } private DefaultBandwidthMeter( @Nullable Context context, SparseArray initialBitrateEstimates, int maxWeight, - Clock clock) { + Clock clock, + boolean resetOnNetworkTypeChange) { + this.context = context == null ? null : context.getApplicationContext(); this.initialBitrateEstimates = initialBitrateEstimates; this.eventDispatcher = new EventDispatcher<>(); this.slidingPercentile = new SlidingPercentile(maxWeight); this.clock = clock; - bitrateEstimate = - getInitialBitrateEstimateForNetworkType( - context == null ? C.NETWORK_TYPE_UNKNOWN : Util.getNetworkType(context)); + // Set the initial network type and bitrate estimate + networkType = context == null ? C.NETWORK_TYPE_UNKNOWN : Util.getNetworkType(context); + bitrateEstimate = getInitialBitrateEstimateForNetworkType(networkType); + // Register to receive connectivity actions if possible. + if (context != null && resetOnNetworkTypeChange) { + ConnectivityActionReceiver connectivityActionReceiver = + ConnectivityActionReceiver.getInstance(context); + connectivityActionReceiver.register(/* bandwidthMeter= */ this); + } } /** * Overrides the network type. Handled in the same way as if the meter had detected a change from - * the current network type to the specified network type. + * the current network type to the specified network type internally. * *

Applications should not normally call this method. It is intended for testing purposes. * * @param networkType The overriding network type. */ public synchronized void setNetworkTypeOverride(@C.NetworkType int networkType) { - // TODO: Handle properly as a network change (in same way as non-external network changes). - bitrateEstimate = getInitialBitrateEstimateForNetworkType(networkType); + networkTypeOverride = networkType; + networkTypeOverrideSet = true; + onConnectivityAction(); } @Override @@ -309,14 +346,50 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList || totalBytesTransferred >= BYTES_TRANSFERRED_FOR_ESTIMATE) { bitrateEstimate = (long) slidingPercentile.getPercentile(0.5f); } - notifyBandwidthSample(sampleElapsedTimeMs, sampleBytesTransferred, bitrateEstimate); + maybeNotifyBandwidthSample(sampleElapsedTimeMs, sampleBytesTransferred, bitrateEstimate); sampleStartTimeMs = nowMs; sampleBytesTransferred = 0; } // Else any sample bytes transferred will be carried forward into the next sample. streamCount--; } - private void notifyBandwidthSample(int elapsedMs, long bytesTransferred, long bitrateEstimate) { + private synchronized void onConnectivityAction() { + int networkType = + networkTypeOverrideSet + ? networkTypeOverride + : (context == null ? C.NETWORK_TYPE_UNKNOWN : Util.getNetworkType(context)); + if (this.networkType == networkType) { + return; + } + + this.networkType = networkType; + if (networkType == C.NETWORK_TYPE_OFFLINE + || networkType == C.NETWORK_TYPE_UNKNOWN + || networkType == C.NETWORK_TYPE_OTHER) { + // It's better not to reset the bandwidth meter for these network types. + return; + } + + // Reset the bitrate estimate and report it, along with any bytes transferred. + this.bitrateEstimate = getInitialBitrateEstimateForNetworkType(networkType); + long nowMs = clock.elapsedRealtime(); + int sampleElapsedTimeMs = streamCount > 0 ? (int) (nowMs - sampleStartTimeMs) : 0; + maybeNotifyBandwidthSample(sampleElapsedTimeMs, sampleBytesTransferred, bitrateEstimate); + + // Reset the remainder of the state. + sampleStartTimeMs = nowMs; + sampleBytesTransferred = 0; + totalBytesTransferred = 0; + totalElapsedTimeMs = 0; + slidingPercentile.reset(); + } + + private void maybeNotifyBandwidthSample( + int elapsedMs, long bytesTransferred, long bitrateEstimate) { + if (elapsedMs == 0 && bytesTransferred == 0 && bitrateEstimate == lastReportedBitrateEstimate) { + return; + } + lastReportedBitrateEstimate = bitrateEstimate; eventDispatcher.dispatch( listener -> listener.onBandwidthSample(elapsedMs, bytesTransferred, bitrateEstimate)); } @@ -332,6 +405,70 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList return initialBitrateEstimate; } + /* + * Note: This class only holds a weak reference to DefaultBandwidthMeter instances. It should not + * be made non-static, since doing so adds a strong reference (i.e. DefaultBandwidthMeter.this). + */ + private static class ConnectivityActionReceiver extends BroadcastReceiver { + + @MonotonicNonNull private static ConnectivityActionReceiver staticInstance; + + private final Handler mainHandler; + private final ArrayList> bandwidthMeters; + + public static synchronized ConnectivityActionReceiver getInstance(Context context) { + if (staticInstance == null) { + staticInstance = new ConnectivityActionReceiver(); + IntentFilter filter = new IntentFilter(); + filter.addAction(ConnectivityManager.CONNECTIVITY_ACTION); + context.registerReceiver(staticInstance, filter); + } + return staticInstance; + } + + private ConnectivityActionReceiver() { + mainHandler = new Handler(Looper.getMainLooper()); + bandwidthMeters = new ArrayList<>(); + } + + public synchronized void register(DefaultBandwidthMeter bandwidthMeter) { + removeClearedReferences(); + bandwidthMeters.add(new WeakReference<>(bandwidthMeter)); + // Simulate an initial update on the main thread (like the sticky broadcast we'd receive if + // we were to register a separate broadcast receiver for each bandwidth meter). + mainHandler.post(() -> updateBandwidthMeter(bandwidthMeter)); + } + + @Override + public synchronized void onReceive(Context context, Intent intent) { + if (isInitialStickyBroadcast()) { + return; + } + removeClearedReferences(); + for (int i = 0; i < bandwidthMeters.size(); i++) { + WeakReference bandwidthMeterReference = bandwidthMeters.get(i); + DefaultBandwidthMeter bandwidthMeter = bandwidthMeterReference.get(); + if (bandwidthMeter != null) { + updateBandwidthMeter(bandwidthMeter); + } + } + } + + private void updateBandwidthMeter(DefaultBandwidthMeter bandwidthMeter) { + bandwidthMeter.onConnectivityAction(); + } + + private void removeClearedReferences() { + for (int i = bandwidthMeters.size() - 1; i >= 0; i--) { + WeakReference bandwidthMeterReference = bandwidthMeters.get(i); + DefaultBandwidthMeter bandwidthMeter = bandwidthMeterReference.get(); + if (bandwidthMeter == null) { + bandwidthMeters.remove(i); + } + } + } + } + private static Map createInitialBitrateCountryGroupAssignment() { HashMap countryGroupAssignment = new HashMap<>(); countryGroupAssignment.put("AD", new int[] {1, 0, 0, 0}); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/SlidingPercentile.java b/library/core/src/main/java/com/google/android/exoplayer2/util/SlidingPercentile.java index f9be1a53b2..c9c21023c3 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/SlidingPercentile.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/SlidingPercentile.java @@ -65,6 +65,14 @@ public class SlidingPercentile { currentSortOrder = SORT_ORDER_NONE; } + /** Resets the sliding percentile. */ + public void reset() { + samples.clear(); + currentSortOrder = SORT_ORDER_NONE; + nextSampleIndex = 0; + totalWeight = 0; + } + /** * Adds a new weighted value. * diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/Util.java b/library/core/src/main/java/com/google/android/exoplayer2/util/Util.java index 116ad860e0..410e1c174c 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/Util.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/Util.java @@ -1652,7 +1652,7 @@ public final class Util { return getMobileNetworkType(networkInfo); case ConnectivityManager.TYPE_ETHERNET: return C.NETWORK_TYPE_ETHERNET; - default: // Ethernet, VPN, Bluetooth, Dummy. + default: // VPN, Bluetooth, Dummy. return C.NETWORK_TYPE_OTHER; } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeterTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeterTest.java index ebdb45909b..ec5d82e712 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeterTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeterTest.java @@ -15,15 +15,18 @@ */ package com.google.android.exoplayer2.upstream; -import static android.Manifest.permission.ACCESS_NETWORK_STATE; import static com.google.common.truth.Truth.assertThat; import android.content.Context; import android.net.ConnectivityManager; import android.net.NetworkInfo; import android.net.NetworkInfo.DetailedState; +import android.net.Uri; import android.telephony.TelephonyManager; import com.google.android.exoplayer2.C; +import com.google.android.exoplayer2.testutil.FakeClock; +import com.google.android.exoplayer2.testutil.FakeDataSource; +import java.util.Random; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -36,6 +39,7 @@ import org.robolectric.shadows.ShadowNetworkInfo; @RunWith(RobolectricTestRunner.class) public final class DefaultBandwidthMeterTest { + private static final int SIMULATED_TRANSFER_COUNT = 100; private static final String FAST_COUNTRY_ISO = "EE"; private static final String SLOW_COUNTRY_ISO = "PG"; @@ -496,6 +500,44 @@ public final class DefaultBandwidthMeterTest { } @Test + public void networkTypeOverride_updatesBitrateEstimate() { + setActiveNetworkInfo(networkInfoEthernet); + DefaultBandwidthMeter bandwidthMeter = + new DefaultBandwidthMeter.Builder(RuntimeEnvironment.application).build(); + long initialEstimateEthernet = bandwidthMeter.getBitrateEstimate(); + + bandwidthMeter.setNetworkTypeOverride(C.NETWORK_TYPE_2G); + long initialEstimate2g = bandwidthMeter.getBitrateEstimate(); + + assertThat(initialEstimateEthernet).isGreaterThan(initialEstimate2g); + } + + @Test + public void networkTypeOverride_doesFullReset() { + // Simulate transfers for an ethernet connection. + setActiveNetworkInfo(networkInfoEthernet); + FakeClock clock = new FakeClock(/* initialTimeMs= */ 0); + DefaultBandwidthMeter bandwidthMeter = + new DefaultBandwidthMeter.Builder(RuntimeEnvironment.application).setClock(clock).build(); + long[] bitrateEstimatesWithNewInstance = simulateTransfers(bandwidthMeter, clock); + + // Create a new instance and seed with some transfers. + setActiveNetworkInfo(networkInfo2g); + bandwidthMeter = + new DefaultBandwidthMeter.Builder(RuntimeEnvironment.application).setClock(clock).build(); + simulateTransfers(bandwidthMeter, clock); + + // Override the network type to ethernet and simulate transfers again. + bandwidthMeter.setNetworkTypeOverride(C.NETWORK_TYPE_ETHERNET); + long[] bitrateEstimatesAfterReset = simulateTransfers(bandwidthMeter, clock); + + // If overriding the network type fully reset the bandwidth meter, we expect the bitrate + // estimates generated during simulation to be the same. + assertThat(bitrateEstimatesAfterReset).isEqualTo(bitrateEstimatesWithNewInstance); + } + + @Test + @SuppressWarnings("deprecation") public void defaultInitialBitrateEstimate_withoutContext_isReasonable() { DefaultBandwidthMeter bandwidthMeterWithBuilder = new DefaultBandwidthMeter.Builder(/* context= */ null).build(); @@ -510,17 +552,6 @@ public final class DefaultBandwidthMeterTest { assertThat(initialEstimateWithoutBuilder).isLessThan(50_000_000L); } - @Test - public void defaultInitialBitrateEstimate_withoutAccessNetworkStatePermission_isReasonable() { - Shadows.shadowOf(RuntimeEnvironment.application).denyPermissions(ACCESS_NETWORK_STATE); - DefaultBandwidthMeter bandwidthMeter = - new DefaultBandwidthMeter.Builder(RuntimeEnvironment.application).build(); - long initialEstimate = bandwidthMeter.getBitrateEstimate(); - - assertThat(initialEstimate).isGreaterThan(100_000L); - assertThat(initialEstimate).isLessThan(50_000_000L); - } - private void setActiveNetworkInfo(NetworkInfo networkInfo) { Shadows.shadowOf(connectivityManager).setActiveNetworkInfo(networkInfo); } @@ -528,4 +559,23 @@ public final class DefaultBandwidthMeterTest { private void setNetworkCountryIso(String countryIso) { Shadows.shadowOf(telephonyManager).setNetworkCountryIso(countryIso); } + + private static long[] simulateTransfers(DefaultBandwidthMeter bandwidthMeter, FakeClock clock) { + long[] bitrateEstimates = new long[SIMULATED_TRANSFER_COUNT]; + Random random = new Random(/* seed= */ 0); + DataSource dataSource = new FakeDataSource(); + DataSpec dataSpec = new DataSpec(Uri.parse("https://dummy.com")); + for (int i = 0; i < SIMULATED_TRANSFER_COUNT; i++) { + bandwidthMeter.onTransferStart(dataSource, dataSpec, /* isNetwork= */ true); + clock.advanceTime(random.nextInt(/* bound= */ 5000)); + bandwidthMeter.onBytesTransferred( + dataSource, + dataSpec, + /* isNetwork= */ true, + /* bytes= */ random.nextInt(5 * 1024 * 1024)); + bandwidthMeter.onTransferEnd(dataSource, dataSpec, /* isNetwork= */ true); + bitrateEstimates[i] = bandwidthMeter.getBitrateEstimate(); + } + return bitrateEstimates; + } }