From 28434044122e8e408ee357c2dd0f63b1e10c1cdc Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Thu, 24 Jan 2019 17:19:51 +0000 Subject: [PATCH 01/23] Move parseSelectionFlags with the rest of the parse{attribute} methods PiperOrigin-RevId: 230734189 --- .../hls/playlist/HlsPlaylistParser.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/HlsPlaylistParser.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/HlsPlaylistParser.java index 242711431c..7dba626e70 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/HlsPlaylistParser.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/HlsPlaylistParser.java @@ -426,21 +426,6 @@ public final class HlsPlaylistParser implements ParsingLoadable.Parser variableDefinitions) throws ParserException { String keyFormatVersions = From c176789927985183d292482c31eda760dd31f978 Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Thu, 24 Jan 2019 18:12:27 +0000 Subject: [PATCH 02/23] Parse CHANNELS attribute from EXT-X-MEDIA PiperOrigin-RevId: 230743198 --- RELEASENOTES.md | 5 ++++ .../hls/playlist/HlsPlaylistParser.java | 11 ++++++++- .../playlist/HlsMasterPlaylistParserTest.java | 23 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 68794310bd..7334e0a2c7 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,5 +1,10 @@ + # Release notes # +### 2.9.5 ### + +* HLS: Parse `CHANNELS` attribute from `EXT-X-MEDIA`. + ### 2.9.4 ### * IMA extension: Clear ads loader listeners on release diff --git a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/HlsPlaylistParser.java b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/HlsPlaylistParser.java index 7dba626e70..9e13d6fa0f 100644 --- a/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/HlsPlaylistParser.java +++ b/library/hls/src/main/java/com/google/android/exoplayer2/source/hls/playlist/HlsPlaylistParser.java @@ -101,6 +101,7 @@ public final class HlsPlaylistParser implements ParsingLoadable.Parser variableDefinitions) { + String channelsString = parseOptionalStringAttr(line, REGEX_CHANNELS, variableDefinitions); + return channelsString != null + ? Integer.parseInt(Util.splitAtFirst(channelsString, "/")[0]) + : Format.NO_VALUE; + } + private static @Nullable SchemeData parsePlayReadySchemeData( String line, Map variableDefinitions) throws ParserException { String keyFormatVersions = diff --git a/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/playlist/HlsMasterPlaylistParserTest.java b/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/playlist/HlsMasterPlaylistParserTest.java index 9701171ce9..8b69ba0db2 100644 --- a/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/playlist/HlsMasterPlaylistParserTest.java +++ b/library/hls/src/test/java/com/google/android/exoplayer2/source/hls/playlist/HlsMasterPlaylistParserTest.java @@ -81,6 +81,18 @@ public class HlsMasterPlaylistParserTest { + "CODECS=\"mp4a.40.2,avc1.66.30\",RESOLUTION=304x128\n" + "http://example.com/low.m3u8\n"; + private static final String PLAYLIST_WITH_CHANNELS_ATTRIBUTE = + " #EXTM3U \n" + + "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"audio\",CHANNELS=\"6\",NAME=\"Eng6\"," + + "URI=\"something.m3u8\"\n" + + "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"audio\",CHANNELS=\"2/6\",NAME=\"Eng26\"," + + "URI=\"something2.m3u8\"\n" + + "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"audio\",NAME=\"Eng\"," + + "URI=\"something3.m3u8\"\n" + + "#EXT-X-STREAM-INF:BANDWIDTH=1280000," + + "CODECS=\"mp4a.40.2,avc1.66.30\",AUDIO=\"audio\",RESOLUTION=304x128\n" + + "http://example.com/low.m3u8\n"; + private static final String PLAYLIST_WITHOUT_CC = " #EXTM3U \n" + "#EXT-X-MEDIA:TYPE=CLOSED-CAPTIONS," @@ -216,6 +228,17 @@ public class HlsMasterPlaylistParserTest { assertThat(closedCaptionFormat.language).isEqualTo("es"); } + @Test + public void testPlaylistWithChannelsAttribute() throws IOException { + HlsMasterPlaylist playlist = + parseMasterPlaylist(PLAYLIST_URI, PLAYLIST_WITH_CHANNELS_ATTRIBUTE); + List audios = playlist.audios; + assertThat(audios).hasSize(3); + assertThat(audios.get(0).format.channelCount).isEqualTo(6); + assertThat(audios.get(1).format.channelCount).isEqualTo(2); + assertThat(audios.get(2).format.channelCount).isEqualTo(Format.NO_VALUE); + } + @Test public void testPlaylistWithoutClosedCaptions() throws IOException { HlsMasterPlaylist playlist = parseMasterPlaylist(PLAYLIST_URI, PLAYLIST_WITHOUT_CC); From aec2b19c7d5853381043b6189c2771e4e755468d Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Fri, 25 Jan 2019 12:07:37 +0000 Subject: [PATCH 03/23] Update IMA to 3.10.6 This brings in a memory leak fix. Issue: #4114 PiperOrigin-RevId: 230880521 --- extensions/ima/build.gradle | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/ima/build.gradle b/extensions/ima/build.gradle index 22196ff3ab..4d6302c898 100644 --- a/extensions/ima/build.gradle +++ b/extensions/ima/build.gradle @@ -31,13 +31,13 @@ android { } dependencies { - api 'com.google.ads.interactivemedia.v3:interactivemedia:3.10.2' + api 'com.google.ads.interactivemedia.v3:interactivemedia:3.10.6' implementation project(modulePrefix + 'library-core') - implementation 'com.google.android.gms:play-services-ads:17.1.1' + implementation 'com.google.android.gms:play-services-ads:17.1.2' // These dependencies are necessary to force the supportLibraryVersion of // com.android.support:support-v4 and com.android.support:customtabs to be // used. Else older versions are used, for example via: - // com.google.android.gms:play-services-ads:17.1.1 + // com.google.android.gms:play-services-ads:17.1.2 // |-- com.android.support:customtabs:26.1.0 implementation 'com.android.support:support-v4:' + supportLibraryVersion implementation 'com.android.support:customtabs:' + supportLibraryVersion From a96d26dbc1d6645298440710b3d23abcad64eaaf Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 28 Jan 2019 08:52:42 +0000 Subject: [PATCH 04/23] Clarify LoopingMediaSource documentation PiperOrigin-RevId: 231171425 --- .../google/android/exoplayer2/source/LoopingMediaSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/LoopingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/LoopingMediaSource.java index 281465eec3..b5a81283fd 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/LoopingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/LoopingMediaSource.java @@ -31,7 +31,7 @@ import java.util.Map; * Loops a {@link MediaSource} a specified number of times. * *

Note: To loop a {@link MediaSource} indefinitely, it is usually better to use {@link - * ExoPlayer#setRepeatMode(int)}. + * ExoPlayer#setRepeatMode(int)} instead of this class. */ public final class LoopingMediaSource extends CompositeMediaSource { From 21e593a14d54a5ddc42dd662556d2e3760575f9c Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Mon, 28 Jan 2019 09:02:05 +0000 Subject: [PATCH 05/23] Fix float output capability check Float output is only possible from API 21, but the high-res int to float conversion path was checking for 32-bit PCM not float output capability. PiperOrigin-RevId: 231172495 --- .../com/google/android/exoplayer2/audio/DefaultAudioSink.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/audio/DefaultAudioSink.java b/library/core/src/main/java/com/google/android/exoplayer2/audio/DefaultAudioSink.java index 429510bcaf..7ee25d07d1 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/audio/DefaultAudioSink.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/audio/DefaultAudioSink.java @@ -419,7 +419,7 @@ public final class DefaultAudioSink implements AudioSink { isInputPcm = Util.isEncodingLinearPcm(inputEncoding); shouldConvertHighResIntPcmToFloat = enableConvertHighResIntPcmToFloat - && supportsOutput(channelCount, C.ENCODING_PCM_32BIT) + && supportsOutput(channelCount, C.ENCODING_PCM_FLOAT) && Util.isEncodingHighResolutionIntegerPcm(inputEncoding); if (isInputPcm) { pcmFrameSize = Util.getPcmFrameSize(inputEncoding, channelCount); From 4acdc8d0b7363be550ebb88874fe4a399516c42c Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Tue, 29 Jan 2019 16:19:39 +0000 Subject: [PATCH 06/23] Fix buffered position when loading has not completed If there is data after the last samples in the container, we may request continue loading after the last samples have been read but before the load has completed. In this situation the buffered position is returned as Long.MAX_VALUE, which prevents continuing loading, yet the media period is not treated as fully buffered because its buffered position is not C.TIME_END_OF_SOURCE. PiperOrigin-RevId: 231406964 --- .../android/exoplayer2/source/ExtractorMediaPeriod.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java index 823901af2a..76a5428a12 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java @@ -346,10 +346,9 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } else if (isPendingReset()) { return pendingResetPositionUs; } - long largestQueuedTimestampUs = C.TIME_UNSET; + long largestQueuedTimestampUs = Long.MAX_VALUE; if (haveAudioVideoTracks) { // Ignore non-AV tracks, which may be sparse or poorly interleaved. - largestQueuedTimestampUs = Long.MAX_VALUE; int trackCount = sampleQueues.length; for (int i = 0; i < trackCount; i++) { if (trackIsAudioVideoFlags[i] && !sampleQueues[i].isLastSampleQueued()) { @@ -358,7 +357,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } } } - if (largestQueuedTimestampUs == C.TIME_UNSET) { + if (largestQueuedTimestampUs == Long.MAX_VALUE) { largestQueuedTimestampUs = getLargestQueuedTimestampUs(); } return largestQueuedTimestampUs == Long.MIN_VALUE ? lastSeekPositionUs From e5bf47e8cfba66209a12d2e15312d66cc14dac5d Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Wed, 30 Jan 2019 20:05:16 +0000 Subject: [PATCH 07/23] Fix issue delivering events to PlayerView OnClickListener --- .../java/com/google/android/exoplayer2/ui/PlayerView.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java index 83f5b70cbb..f7f509dfa7 100644 --- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java +++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java @@ -1035,6 +1035,12 @@ public class PlayerView extends FrameLayout { if (ev.getActionMasked() != MotionEvent.ACTION_DOWN) { return false; } + return performClick(); + } + + @Override + public boolean performClick() { + super.performClick(); return toggleControllerVisibility(); } From 92e60d2edb575cf19f385bfc7d5af212401bddc0 Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Mon, 21 Jan 2019 10:18:01 +0000 Subject: [PATCH 08/23] Add DefaultTsPayloadReaderFactory flag to ignore HDMV DTS streams Prevents collisions with SCTE-35 subtitles. Issue:#5330 PiperOrigin-RevId: 230195494 --- .../ts/DefaultTsPayloadReaderFactory.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ts/DefaultTsPayloadReaderFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ts/DefaultTsPayloadReaderFactory.java index a5506e2cfb..88805d9362 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ts/DefaultTsPayloadReaderFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ts/DefaultTsPayloadReaderFactory.java @@ -50,7 +50,8 @@ public final class DefaultTsPayloadReaderFactory implements TsPayloadReader.Fact FLAG_IGNORE_H264_STREAM, FLAG_DETECT_ACCESS_UNITS, FLAG_IGNORE_SPLICE_INFO_STREAM, - FLAG_OVERRIDE_CAPTION_DESCRIPTORS + FLAG_OVERRIDE_CAPTION_DESCRIPTORS, + FLAG_IGNORE_HDMV_DTS_STREAM }) public @interface Flags {} @@ -86,6 +87,12 @@ public final class DefaultTsPayloadReaderFactory implements TsPayloadReader.Fact * closedCaptionFormats} will be ignored if the PMT contains closed captions service descriptors. */ public static final int FLAG_OVERRIDE_CAPTION_DESCRIPTORS = 1 << 5; + /** + * Prevents the creation of {@link DtsReader} instances when receiving {@link + * TsExtractor#TS_STREAM_TYPE_HDMV_DTS} as stream type. Enabling this flag prevents a stream type + * collision between HDMV DTS audio and SCTE-35 subtitles. + */ + public static final int FLAG_IGNORE_HDMV_DTS_STREAM = 1 << 6; private static final int DESCRIPTOR_TAG_CAPTION_SERVICE = 0x86; @@ -142,8 +149,12 @@ public final class DefaultTsPayloadReaderFactory implements TsPayloadReader.Fact case TsExtractor.TS_STREAM_TYPE_AC3: case TsExtractor.TS_STREAM_TYPE_E_AC3: return new PesReader(new Ac3Reader(esInfo.language)); - case TsExtractor.TS_STREAM_TYPE_DTS: case TsExtractor.TS_STREAM_TYPE_HDMV_DTS: + if (isSet(FLAG_IGNORE_HDMV_DTS_STREAM)) { + return null; + } + // Fall through. + case TsExtractor.TS_STREAM_TYPE_DTS: return new PesReader(new DtsReader(esInfo.language)); case TsExtractor.TS_STREAM_TYPE_H262: return new PesReader(new H262Reader(buildUserDataReader(esInfo))); From e553fb3f66e0b1b0dde584bb322cd957e4b77c5d Mon Sep 17 00:00:00 2001 From: olly Date: Wed, 30 Jan 2019 20:22:33 +0000 Subject: [PATCH 09/23] Update OkHttp and Cronet dependencies PiperOrigin-RevId: 231644908 --- extensions/cronet/build.gradle | 2 +- extensions/okhttp/build.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/cronet/build.gradle b/extensions/cronet/build.gradle index 7d8c217b58..520edfe1d1 100644 --- a/extensions/cronet/build.gradle +++ b/extensions/cronet/build.gradle @@ -30,7 +30,7 @@ android { } dependencies { - api 'org.chromium.net:cronet-embedded:66.3359.158' + api 'org.chromium.net:cronet-embedded:71.3578.98' implementation project(modulePrefix + 'library-core') implementation 'com.android.support:support-annotations:' + supportLibraryVersion testImplementation project(modulePrefix + 'library') diff --git a/extensions/okhttp/build.gradle b/extensions/okhttp/build.gradle index 4e6b11c495..78825a6277 100644 --- a/extensions/okhttp/build.gradle +++ b/extensions/okhttp/build.gradle @@ -34,7 +34,7 @@ dependencies { implementation project(modulePrefix + 'library-core') implementation 'com.android.support:support-annotations:' + supportLibraryVersion compileOnly 'org.checkerframework:checker-qual:' + checkerframeworkVersion - api 'com.squareup.okhttp3:okhttp:3.11.0' + api 'com.squareup.okhttp3:okhttp:3.12.1' } ext { From 9997bb41ab3b3da128647fa8883721867290972b Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Wed, 30 Jan 2019 22:31:33 +0000 Subject: [PATCH 10/23] Update release notes --- RELEASENOTES.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 7334e0a2c7..ac1962e6d2 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -3,7 +3,14 @@ ### 2.9.5 ### -* HLS: Parse `CHANNELS` attribute from `EXT-X-MEDIA`. +* HLS: Parse `CHANNELS` attribute from `EXT-X-MEDIA` tag. +* ExtractorMediaSource: Fix issue that could cause the player to get stuck + buffering at the end of the media. +* PlayerView: Fix issue preventing `OnClickListener` from receiving events + ([#5433](https://github.com/google/ExoPlayer/issues/5433)). +* IMA extension: Upgrade IMA dependency to 3.10.6. +* Cronet extension: Upgrade Cronet dependency to 71.3578.98. +* OkHttp extension: Upgrade OkHttp dependency to 3.12.1. ### 2.9.4 ### From 4a946550c9dd9aeb45c6f2475e584a8b29ceb7ad Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Wed, 30 Jan 2019 22:33:12 +0000 Subject: [PATCH 11/23] Update version --- constants.gradle | 4 ++-- .../com/google/android/exoplayer2/ExoPlayerLibraryInfo.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/constants.gradle b/constants.gradle index 716ddbadba..16fb3420ce 100644 --- a/constants.gradle +++ b/constants.gradle @@ -13,8 +13,8 @@ // limitations under the License. project.ext { // ExoPlayer version and version code. - releaseVersion = '2.9.4' - releaseVersionCode = 2009004 + releaseVersion = '2.9.5' + releaseVersionCode = 2009005 // Important: ExoPlayer specifies a minSdkVersion of 14 because various // components provided by the library may be of use on older devices. // However, please note that the core media playback functionality provided diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java index 36723c5d73..e3a2e1cd27 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java @@ -29,11 +29,11 @@ public final class ExoPlayerLibraryInfo { /** The version of the library expressed as a string, for example "1.2.3". */ // Intentionally hardcoded. Do not derive from other constants (e.g. VERSION_INT) or vice versa. - public static final String VERSION = "2.9.4"; + public static final String VERSION = "2.9.5"; /** The version of the library expressed as {@code "ExoPlayerLib/" + VERSION}. */ // Intentionally hardcoded. Do not derive from other constants (e.g. VERSION) or vice versa. - public static final String VERSION_SLASHY = "ExoPlayerLib/2.9.4"; + public static final String VERSION_SLASHY = "ExoPlayerLib/2.9.5"; /** * The version of the library expressed as an integer, for example 1002003. @@ -43,7 +43,7 @@ public final class ExoPlayerLibraryInfo { * integer version 123045006 (123-045-006). */ // Intentionally hardcoded. Do not derive from other constants (e.g. VERSION) or vice versa. - public static final int VERSION_INT = 2009004; + public static final int VERSION_INT = 2009005; /** * Whether the library was compiled with {@link com.google.android.exoplayer2.util.Assertions} From 7a819cba656a98c7a7ab97f2ae17c4c23be47b7d Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Wed, 30 Jan 2019 22:35:19 +0000 Subject: [PATCH 12/23] Remove stray line --- RELEASENOTES.md | 1 - 1 file changed, 1 deletion(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index ac1962e6d2..12b4bc42b5 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,4 +1,3 @@ - # Release notes # ### 2.9.5 ### From ba2ef0be9449a8a8ac966ec26a13b6d3375a090a Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 31 Jan 2019 11:45:08 +0000 Subject: [PATCH 13/23] Add additional device to output surface workaround Issue: #4468 PiperOrigin-RevId: 231759438 --- .../android/exoplayer2/video/MediaCodecVideoRenderer.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/video/MediaCodecVideoRenderer.java b/library/core/src/main/java/com/google/android/exoplayer2/video/MediaCodecVideoRenderer.java index 388aa29ce9..5b35ee946a 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/video/MediaCodecVideoRenderer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/video/MediaCodecVideoRenderer.java @@ -1319,8 +1319,9 @@ public class MediaCodecVideoRenderer extends MediaCodecRenderer { } synchronized (MediaCodecVideoRenderer.class) { if (!evaluatedDeviceNeedsSetOutputSurfaceWorkaround) { - if (Util.SDK_INT <= 27 && "dangal".equals(Util.DEVICE)) { - // Dangal is affected on API level 27: https://github.com/google/ExoPlayer/issues/5169. + if (Util.SDK_INT <= 27 && ("dangal".equals(Util.DEVICE) || "HWEML".equals(Util.DEVICE))) { + // A small number of devices are affected on API level 27: + // https://github.com/google/ExoPlayer/issues/5169. deviceNeedsSetOutputSurfaceWorkaround = true; } else if (Util.SDK_INT >= 27) { // In general, devices running API level 27 or later should be unaffected. Do nothing. From 41689fdef7956409328e724fde5339fab092c65c Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Thu, 31 Jan 2019 14:03:11 +0000 Subject: [PATCH 14/23] Remove remainder of skip button focus hack PiperOrigin-RevId: 231772920 --- .../java/com/google/android/exoplayer2/ui/PlayerView.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java index f7f509dfa7..9742d0005a 100644 --- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java +++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java @@ -758,10 +758,6 @@ public class PlayerView extends FrameLayout { @Override public boolean dispatchKeyEvent(KeyEvent event) { if (player != null && player.isPlayingAd()) { - // Focus any overlay UI now, in case it's provided by a WebView whose contents may update - // dynamically. This is needed to make the "Skip ad" button focused on Android TV when using - // IMA [Internal: b/62371030]. - overlayFrameLayout.requestFocus(); return super.dispatchKeyEvent(event); } boolean isDpadWhenControlHidden = From f8772fda21135e519f05d3e168e95eadb54d0c1b Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 4 Feb 2019 16:11:26 +0000 Subject: [PATCH 15/23] Wider fix for OMX.SEC.mp3.dec issue Issue: #4519 PiperOrigin-RevId: 232299233 --- RELEASENOTES.md | 2 + .../exoplayer2/mediacodec/MediaCodecUtil.java | 53 +++++++++---------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 12b4bc42b5..cff58e4b9a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -10,6 +10,8 @@ * IMA extension: Upgrade IMA dependency to 3.10.6. * Cronet extension: Upgrade Cronet dependency to 71.3578.98. * OkHttp extension: Upgrade OkHttp dependency to 3.12.1. +* MP3: Wider fix for issue where streams would play twice on some Samsung + devices ([#4519](https://github.com/google/ExoPlayer/issues/4519)). ### 2.9.4 ### diff --git a/library/core/src/main/java/com/google/android/exoplayer2/mediacodec/MediaCodecUtil.java b/library/core/src/main/java/com/google/android/exoplayer2/mediacodec/MediaCodecUtil.java index 9ae50179c3..f09cae9949 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/mediacodec/MediaCodecUtil.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/mediacodec/MediaCodecUtil.java @@ -59,8 +59,6 @@ public final class MediaCodecUtil { private static final String TAG = "MediaCodecUtil"; private static final Pattern PROFILE_PATTERN = Pattern.compile("^\\D?(\\d+)$"); - private static final RawAudioCodecComparator RAW_AUDIO_CODEC_COMPARATOR = - new RawAudioCodecComparator(); private static final HashMap> decoderInfosCache = new HashMap<>(); @@ -312,32 +310,6 @@ public final class MediaCodecUtil { return false; } - // Work around https://github.com/google/ExoPlayer/issues/398. - if (Util.SDK_INT < 18 && "OMX.SEC.MP3.Decoder".equals(name)) { - return false; - } - - // Work around https://github.com/google/ExoPlayer/issues/4519. - if ("OMX.SEC.mp3.dec".equals(name) - && (Util.MODEL.startsWith("GT-I9152") - || Util.MODEL.startsWith("GT-I9515") - || Util.MODEL.startsWith("GT-P5220") - || Util.MODEL.startsWith("GT-S7580") - || Util.MODEL.startsWith("SM-G350") - || Util.MODEL.startsWith("SM-G386") - || Util.MODEL.startsWith("SM-T231") - || Util.MODEL.startsWith("SM-T530") - || Util.MODEL.startsWith("SCH-I535") - || Util.MODEL.startsWith("SPH-L710"))) { - return false; - } - if ("OMX.brcm.audio.mp3.decoder".equals(name) - && (Util.MODEL.startsWith("GT-I9152") - || Util.MODEL.startsWith("GT-S7580") - || Util.MODEL.startsWith("SM-G350"))) { - return false; - } - // Work around https://github.com/google/ExoPlayer/issues/1528 and // https://github.com/google/ExoPlayer/issues/3171. if (Util.SDK_INT < 18 && "OMX.MTK.AUDIO.DECODER.AAC".equals(name) @@ -424,7 +396,18 @@ public final class MediaCodecUtil { */ private static void applyWorkarounds(String mimeType, List decoderInfos) { if (MimeTypes.AUDIO_RAW.equals(mimeType)) { - Collections.sort(decoderInfos, RAW_AUDIO_CODEC_COMPARATOR); + Collections.sort(decoderInfos, new RawAudioCodecComparator()); + } else if (Util.SDK_INT < 21 && decoderInfos.size() > 1) { + String firstCodecName = decoderInfos.get(0).name; + if ("OMX.SEC.mp3.dec".equals(firstCodecName) + || "OMX.SEC.MP3.Decoder".equals(firstCodecName) + || "OMX.brcm.audio.mp3.decoder".equals(firstCodecName)) { + // Prefer OMX.google codecs over OMX.SEC.mp3.dec, OMX.SEC.MP3.Decoder and + // OMX.brcm.audio.mp3.decoder on older devices. See: + // https://github.com/google/ExoPlayer/issues/398 and + // https://github.com/google/ExoPlayer/issues/4519. + Collections.sort(decoderInfos, new PreferOmxGoogleCodecComparator()); + } } } @@ -730,6 +713,18 @@ public final class MediaCodecUtil { } } + /** Comparator for preferring OMX.google media codecs. */ + private static final class PreferOmxGoogleCodecComparator implements Comparator { + @Override + public int compare(MediaCodecInfo a, MediaCodecInfo b) { + return scoreMediaCodecInfo(a) - scoreMediaCodecInfo(b); + } + + private static int scoreMediaCodecInfo(MediaCodecInfo mediaCodecInfo) { + return mediaCodecInfo.name.startsWith("OMX.google") ? -1 : 0; + } + } + static { AVC_PROFILE_NUMBER_TO_CONST = new SparseIntArray(); AVC_PROFILE_NUMBER_TO_CONST.put(66, CodecProfileLevel.AVCProfileBaseline); From 1f476ecef077639a2407c34c2df974a4d2284c3c Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 4 Feb 2019 17:39:35 +0000 Subject: [PATCH 16/23] Add missing removeCallbacksAndMessages to RobolectricUtil. The Util class already handles removeMessages calls but so far ignored calls to removeCallbacksAndMessages. PiperOrigin-RevId: 232312458 --- .../exoplayer2/testutil/RobolectricUtil.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/RobolectricUtil.java b/testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/RobolectricUtil.java index dc7781fd90..1e7f09bacf 100644 --- a/testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/RobolectricUtil.java +++ b/testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/RobolectricUtil.java @@ -37,6 +37,7 @@ import org.robolectric.shadows.ShadowMessageQueue; public final class RobolectricUtil { private static final AtomicLong sequenceNumberGenerator = new AtomicLong(0); + private static final int ANY_MESSAGE = Integer.MIN_VALUE; private RobolectricUtil() {} @@ -110,7 +111,8 @@ public final class RobolectricUtil { boolean isRemoved = false; for (RemovedMessage removedMessage : removedMessages) { if (removedMessage.handler == target - && removedMessage.what == pendingMessage.message.what + && (removedMessage.what == ANY_MESSAGE + || removedMessage.what == pendingMessage.message.what) && (removedMessage.object == null || removedMessage.object == pendingMessage.message.obj) && pendingMessage.sequenceNumber < removedMessage.sequenceNumber) { @@ -179,6 +181,15 @@ public final class RobolectricUtil { ((CustomLooper) shadowOf(looper)).removeMessages(handler, what, object); } } + + @Implementation + public void removeCallbacksAndMessages(Handler handler, Object object) { + Looper looper = ShadowLooper.getLooperForThread(looperThread); + if (shadowOf(looper) instanceof CustomLooper + && shadowOf(looper) != ShadowLooper.getShadowMainLooper()) { + ((CustomLooper) shadowOf(looper)).removeMessages(handler, ANY_MESSAGE, object); + } + } } private static final class PendingMessage implements Comparable { From fb81d629f0ee34a3bd4bb49cc7680d184298c297 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 4 Feb 2019 19:35:06 +0000 Subject: [PATCH 17/23] Merge #5462: Making easier to set the playClearSampleWithoutKeys to renderers Imported from GitHub PR https://github.com/google/ExoPlayer/pull/5462 Pull request for the following issue: #5421 Merge d9d88b079c4ca0533a836b2715a65b924babbb89 into a73819162751116acd3863cf5473b0ff78fac805 PiperOrigin-RevId: 232335113 --- .../exoplayer2/DefaultRenderersFactory.java | 197 +++++++++++++----- .../android/exoplayer2/ExoPlayerFactory.java | 7 +- .../testutil/DebugRenderersFactory.java | 56 +++-- 3 files changed, 195 insertions(+), 65 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/DefaultRenderersFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/DefaultRenderersFactory.java index cc16c43b05..50832dd5af 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/DefaultRenderersFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/DefaultRenderersFactory.java @@ -16,6 +16,7 @@ package com.google.android.exoplayer2; import android.content.Context; +import android.media.MediaCodec; import android.os.Handler; import android.os.Looper; import android.support.annotation.IntDef; @@ -85,15 +86,18 @@ public class DefaultRenderersFactory implements RenderersFactory { protected static final int MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY = 50; private final Context context; - private final @Nullable DrmSessionManager drmSessionManager; - private final @ExtensionRendererMode int extensionRendererMode; - private final long allowedVideoJoiningTimeMs; + @Nullable private DrmSessionManager drmSessionManager; + @ExtensionRendererMode private int extensionRendererMode; + private long allowedVideoJoiningTimeMs; + private boolean playClearSamplesWithoutKeys; + private MediaCodecSelector mediaCodecSelector; - /** - * @param context A {@link Context}. - */ + /** @param context A {@link Context}. */ public DefaultRenderersFactory(Context context) { - this(context, EXTENSION_RENDERER_MODE_OFF); + this.context = context; + extensionRendererMode = EXTENSION_RENDERER_MODE_OFF; + allowedVideoJoiningTimeMs = DEFAULT_ALLOWED_VIDEO_JOINING_TIME_MS; + mediaCodecSelector = MediaCodecSelector.DEFAULT; } /** @@ -108,19 +112,20 @@ public class DefaultRenderersFactory implements RenderersFactory { } /** - * @param context A {@link Context}. - * @param extensionRendererMode The extension renderer mode, which determines if and how available - * extension renderers are used. Note that extensions must be included in the application - * build for them to be considered available. + * @deprecated Use {@link #DefaultRenderersFactory(Context)} and {@link + * #setExtensionRendererMode(int)}. */ + @Deprecated + @SuppressWarnings("deprecation") public DefaultRenderersFactory( Context context, @ExtensionRendererMode int extensionRendererMode) { this(context, extensionRendererMode, DEFAULT_ALLOWED_VIDEO_JOINING_TIME_MS); } /** - * @deprecated Use {@link #DefaultRenderersFactory(Context, int)} and pass {@link - * DrmSessionManager} directly to {@link SimpleExoPlayer} or {@link ExoPlayerFactory}. + * @deprecated Use {@link #DefaultRenderersFactory(Context)} and {@link + * #setExtensionRendererMode(int)}, and pass {@link DrmSessionManager} directly to {@link + * SimpleExoPlayer} or {@link ExoPlayerFactory}. */ @Deprecated @SuppressWarnings("deprecation") @@ -132,26 +137,22 @@ public class DefaultRenderersFactory implements RenderersFactory { } /** - * @param context A {@link Context}. - * @param extensionRendererMode The extension renderer mode, which determines if and how available - * extension renderers are used. Note that extensions must be included in the application - * build for them to be considered available. - * @param allowedVideoJoiningTimeMs The maximum duration for which video renderers can attempt to - * seamlessly join an ongoing playback. + * @deprecated Use {@link #DefaultRenderersFactory(Context)}, {@link + * #setExtensionRendererMode(int)} and {@link #setAllowedVideoJoiningTimeMs(long)}. */ + @Deprecated + @SuppressWarnings("deprecation") public DefaultRenderersFactory( Context context, @ExtensionRendererMode int extensionRendererMode, long allowedVideoJoiningTimeMs) { - this.context = context; - this.extensionRendererMode = extensionRendererMode; - this.allowedVideoJoiningTimeMs = allowedVideoJoiningTimeMs; - this.drmSessionManager = null; + this(context, null, extensionRendererMode, allowedVideoJoiningTimeMs); } /** - * @deprecated Use {@link #DefaultRenderersFactory(Context, int, long)} and pass {@link - * DrmSessionManager} directly to {@link SimpleExoPlayer} or {@link ExoPlayerFactory}. + * @deprecated Use {@link #DefaultRenderersFactory(Context)}, {@link + * #setExtensionRendererMode(int)} and {@link #setAllowedVideoJoiningTimeMs(long)}, and pass + * {@link DrmSessionManager} directly to {@link SimpleExoPlayer} or {@link ExoPlayerFactory}. */ @Deprecated public DefaultRenderersFactory( @@ -163,6 +164,70 @@ public class DefaultRenderersFactory implements RenderersFactory { this.extensionRendererMode = extensionRendererMode; this.allowedVideoJoiningTimeMs = allowedVideoJoiningTimeMs; this.drmSessionManager = drmSessionManager; + mediaCodecSelector = MediaCodecSelector.DEFAULT; + } + + /** + * Sets the extension renderer mode, which determines if and how available extension renderers are + * used. Note that extensions must be included in the application build for them to be considered + * available. + * + *

The default value is {@link #EXTENSION_RENDERER_MODE_OFF}. + * + * @param extensionRendererMode The extension renderer mode. + * @return This factory, for convenience. + */ + public DefaultRenderersFactory setExtensionRendererMode( + @ExtensionRendererMode int extensionRendererMode) { + this.extensionRendererMode = extensionRendererMode; + return this; + } + + /** + * Sets whether renderers are permitted to play clear regions of encrypted media prior to having + * obtained the keys necessary to decrypt encrypted regions of the media. For encrypted media that + * starts with a short clear region, this allows playback to begin in parallel with key + * acquisition, which can reduce startup latency. + * + *

The default value is {@code false}. + * + * @param playClearSamplesWithoutKeys Whether renderers are permitted to play clear regions of + * encrypted media prior to having obtained the keys necessary to decrypt encrypted regions of + * the media. + * @return This factory, for convenience. + */ + public DefaultRenderersFactory setPlayClearSamplesWithoutKeys( + boolean playClearSamplesWithoutKeys) { + this.playClearSamplesWithoutKeys = playClearSamplesWithoutKeys; + return this; + } + + /** + * Sets a {@link MediaCodecSelector} for use by {@link MediaCodec} based renderers. + * + *

The default value is {@link MediaCodecSelector#DEFAULT}. + * + * @param mediaCodecSelector The {@link MediaCodecSelector}. + * @return This factory, for convenience. + */ + public DefaultRenderersFactory setMediaCodecSelector(MediaCodecSelector mediaCodecSelector) { + this.mediaCodecSelector = mediaCodecSelector; + return this; + } + + /** + * Sets the maximum duration for which video renderers can attempt to seamlessly join an ongoing + * playback. + * + *

The default value is {@link #DEFAULT_ALLOWED_VIDEO_JOINING_TIME_MS}. + * + * @param allowedVideoJoiningTimeMs The maximum duration for which video renderers can attempt to + * seamlessly join an ongoing playback, in milliseconds. + * @return This factory, for convenience. + */ + public DefaultRenderersFactory setAllowedVideoJoiningTimeMs(long allowedVideoJoiningTimeMs) { + this.allowedVideoJoiningTimeMs = allowedVideoJoiningTimeMs; + return this; } @Override @@ -177,10 +242,26 @@ public class DefaultRenderersFactory implements RenderersFactory { drmSessionManager = this.drmSessionManager; } ArrayList renderersList = new ArrayList<>(); - buildVideoRenderers(context, drmSessionManager, allowedVideoJoiningTimeMs, - eventHandler, videoRendererEventListener, extensionRendererMode, renderersList); - buildAudioRenderers(context, drmSessionManager, buildAudioProcessors(), - eventHandler, audioRendererEventListener, extensionRendererMode, renderersList); + buildVideoRenderers( + context, + extensionRendererMode, + mediaCodecSelector, + drmSessionManager, + playClearSamplesWithoutKeys, + eventHandler, + videoRendererEventListener, + allowedVideoJoiningTimeMs, + renderersList); + buildAudioRenderers( + context, + extensionRendererMode, + mediaCodecSelector, + drmSessionManager, + playClearSamplesWithoutKeys, + buildAudioProcessors(), + eventHandler, + audioRendererEventListener, + renderersList); buildTextRenderers(context, textRendererOutput, eventHandler.getLooper(), extensionRendererMode, renderersList); buildMetadataRenderers(context, metadataRendererOutput, eventHandler.getLooper(), @@ -194,27 +275,36 @@ public class DefaultRenderersFactory implements RenderersFactory { * Builds video renderers for use by the player. * * @param context The {@link Context} associated with the player. - * @param drmSessionManager An optional {@link DrmSessionManager}. May be null if the player - * will not be used for DRM protected playbacks. - * @param allowedVideoJoiningTimeMs The maximum duration in milliseconds for which video - * renderers can attempt to seamlessly join an ongoing playback. + * @param extensionRendererMode The extension renderer mode. + * @param mediaCodecSelector A decoder selector. + * @param drmSessionManager An optional {@link DrmSessionManager}. May be null if the player will + * not be used for DRM protected playbacks. + * @param playClearSamplesWithoutKeys Whether renderers are permitted to play clear regions of + * encrypted media prior to having obtained the keys necessary to decrypt encrypted regions of + * the media. * @param eventHandler A handler associated with the main thread's looper. * @param eventListener An event listener. - * @param extensionRendererMode The extension renderer mode. + * @param allowedVideoJoiningTimeMs The maximum duration for which video renderers can attempt to + * seamlessly join an ongoing playback, in milliseconds. * @param out An array to which the built renderers should be appended. */ - protected void buildVideoRenderers(Context context, + protected void buildVideoRenderers( + Context context, + @ExtensionRendererMode int extensionRendererMode, + MediaCodecSelector mediaCodecSelector, @Nullable DrmSessionManager drmSessionManager, - long allowedVideoJoiningTimeMs, Handler eventHandler, - VideoRendererEventListener eventListener, @ExtensionRendererMode int extensionRendererMode, + boolean playClearSamplesWithoutKeys, + Handler eventHandler, + VideoRendererEventListener eventListener, + long allowedVideoJoiningTimeMs, ArrayList out) { out.add( new MediaCodecVideoRenderer( context, - MediaCodecSelector.DEFAULT, + mediaCodecSelector, allowedVideoJoiningTimeMs, drmSessionManager, - /* playClearSamplesWithoutKeys= */ false, + playClearSamplesWithoutKeys, eventHandler, eventListener, MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY)); @@ -261,26 +351,35 @@ public class DefaultRenderersFactory implements RenderersFactory { * Builds audio renderers for use by the player. * * @param context The {@link Context} associated with the player. - * @param drmSessionManager An optional {@link DrmSessionManager}. May be null if the player - * will not be used for DRM protected playbacks. - * @param audioProcessors An array of {@link AudioProcessor}s that will process PCM audio - * buffers before output. May be empty. + * @param extensionRendererMode The extension renderer mode. + * @param mediaCodecSelector A decoder selector. + * @param drmSessionManager An optional {@link DrmSessionManager}. May be null if the player will + * not be used for DRM protected playbacks. + * @param playClearSamplesWithoutKeys Whether renderers are permitted to play clear regions of + * encrypted media prior to having obtained the keys necessary to decrypt encrypted regions of + * the media. + * @param audioProcessors An array of {@link AudioProcessor}s that will process PCM audio buffers + * before output. May be empty. * @param eventHandler A handler to use when invoking event listeners and outputs. * @param eventListener An event listener. - * @param extensionRendererMode The extension renderer mode. * @param out An array to which the built renderers should be appended. */ - protected void buildAudioRenderers(Context context, + protected void buildAudioRenderers( + Context context, + @ExtensionRendererMode int extensionRendererMode, + MediaCodecSelector mediaCodecSelector, @Nullable DrmSessionManager drmSessionManager, - AudioProcessor[] audioProcessors, Handler eventHandler, - AudioRendererEventListener eventListener, @ExtensionRendererMode int extensionRendererMode, + boolean playClearSamplesWithoutKeys, + AudioProcessor[] audioProcessors, + Handler eventHandler, + AudioRendererEventListener eventListener, ArrayList out) { out.add( new MediaCodecAudioRenderer( context, - MediaCodecSelector.DEFAULT, + mediaCodecSelector, drmSessionManager, - /* playClearSamplesWithoutKeys= */ false, + playClearSamplesWithoutKeys, eventHandler, eventListener, AudioCapabilities.getCapabilities(context), diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerFactory.java index 81f4285a08..6c2a6f527c 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerFactory.java @@ -97,7 +97,8 @@ public final class ExoPlayerFactory { LoadControl loadControl, @Nullable DrmSessionManager drmSessionManager, @DefaultRenderersFactory.ExtensionRendererMode int extensionRendererMode) { - RenderersFactory renderersFactory = new DefaultRenderersFactory(context, extensionRendererMode); + RenderersFactory renderersFactory = + new DefaultRenderersFactory(context).setExtensionRendererMode(extensionRendererMode); return newSimpleInstance( context, renderersFactory, trackSelector, loadControl, drmSessionManager); } @@ -127,7 +128,9 @@ public final class ExoPlayerFactory { @DefaultRenderersFactory.ExtensionRendererMode int extensionRendererMode, long allowedVideoJoiningTimeMs) { RenderersFactory renderersFactory = - new DefaultRenderersFactory(context, extensionRendererMode, allowedVideoJoiningTimeMs); + new DefaultRenderersFactory(context) + .setExtensionRendererMode(extensionRendererMode) + .setAllowedVideoJoiningTimeMs(allowedVideoJoiningTimeMs); return newSimpleInstance( context, renderersFactory, trackSelector, loadControl, drmSessionManager); } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DebugRenderersFactory.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DebugRenderersFactory.java index 627b5b72f3..4ea2d7d754 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DebugRenderersFactory.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DebugRenderersFactory.java @@ -20,6 +20,7 @@ import android.content.Context; import android.media.MediaCodec; import android.media.MediaCrypto; import android.os.Handler; +import android.support.annotation.Nullable; import com.google.android.exoplayer2.DefaultRenderersFactory; import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.Format; @@ -37,23 +38,38 @@ import java.util.ArrayList; /** * A debug extension of {@link DefaultRenderersFactory}. Provides a video renderer that performs - * video buffer timestamp assertions. + * video buffer timestamp assertions, and modifies the default value for {@link + * #setAllowedVideoJoiningTimeMs(long)} to be {@code 0}. */ @TargetApi(16) public class DebugRenderersFactory extends DefaultRenderersFactory { public DebugRenderersFactory(Context context) { - super(context, DefaultRenderersFactory.EXTENSION_RENDERER_MODE_OFF, 0); + super(context); + setAllowedVideoJoiningTimeMs(0); } @Override - protected void buildVideoRenderers(Context context, - DrmSessionManager drmSessionManager, long allowedVideoJoiningTimeMs, - Handler eventHandler, VideoRendererEventListener eventListener, - @ExtensionRendererMode int extensionRendererMode, ArrayList out) { - out.add(new DebugMediaCodecVideoRenderer(context, MediaCodecSelector.DEFAULT, - allowedVideoJoiningTimeMs, drmSessionManager, eventHandler, eventListener, - MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY)); + protected void buildVideoRenderers( + Context context, + @ExtensionRendererMode int extensionRendererMode, + MediaCodecSelector mediaCodecSelector, + @Nullable DrmSessionManager drmSessionManager, + boolean playClearSamplesWithoutKeys, + Handler eventHandler, + VideoRendererEventListener eventListener, + long allowedVideoJoiningTimeMs, + ArrayList out) { + out.add( + new DebugMediaCodecVideoRenderer( + context, + mediaCodecSelector, + allowedVideoJoiningTimeMs, + drmSessionManager, + playClearSamplesWithoutKeys, + eventHandler, + eventListener, + MAX_DROPPED_VIDEO_FRAME_COUNT_TO_NOTIFY)); } /** @@ -72,12 +88,24 @@ public class DebugRenderersFactory extends DefaultRenderersFactory { private int minimumInsertIndex; private boolean skipToPositionBeforeRenderingFirstFrame; - public DebugMediaCodecVideoRenderer(Context context, MediaCodecSelector mediaCodecSelector, - long allowedJoiningTimeMs, DrmSessionManager drmSessionManager, - Handler eventHandler, VideoRendererEventListener eventListener, + public DebugMediaCodecVideoRenderer( + Context context, + MediaCodecSelector mediaCodecSelector, + long allowedJoiningTimeMs, + DrmSessionManager drmSessionManager, + boolean playClearSamplesWithoutKeys, + Handler eventHandler, + VideoRendererEventListener eventListener, int maxDroppedFrameCountToNotify) { - super(context, mediaCodecSelector, allowedJoiningTimeMs, drmSessionManager, false, - eventHandler, eventListener, maxDroppedFrameCountToNotify); + super( + context, + mediaCodecSelector, + allowedJoiningTimeMs, + drmSessionManager, + playClearSamplesWithoutKeys, + eventHandler, + eventListener, + maxDroppedFrameCountToNotify); } @Override From 22599a6d6cb581ddc578f485c8ba08c51a4a09a5 Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 4 Jan 2019 17:55:49 +0000 Subject: [PATCH 18/23] Pass Handler together with Runnable callbacks for playlist commands. We currently either use the app thread returned by the player or the thread the commands are called on depending on whether the media source is already prepared or not. This change lets the application decide which callback thread to use. As a side effect, we also don't longer need access the player instance passed to MediaSource.prepare. PiperOrigin-RevId: 227871111 --- RELEASENOTES.md | 2 + .../source/ConcatenatingMediaSource.java | 308 +++++++++++------- .../source/ConcatenatingMediaSourceTest.java | 34 +- 3 files changed, 208 insertions(+), 136 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index cff58e4b9a..271f2d94b9 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,6 +2,8 @@ ### 2.9.5 ### +* Add `Handler` parameter to `ConcatenatingMediaSource` methods which take a + callback `Runnable`. * HLS: Parse `CHANNELS` attribute from `EXT-X-MEDIA` tag. * ExtractorMediaSource: Fix issue that could cause the player to get stuck buffering at the end of the media. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java index c93afdb249..fd567234e9 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java @@ -17,6 +17,7 @@ package com.google.android.exoplayer2.source; import android.os.Handler; import android.os.Message; +import android.support.annotation.GuardedBy; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.util.Pair; @@ -28,6 +29,7 @@ import com.google.android.exoplayer2.source.ShuffleOrder.DefaultShuffleOrder; import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.util.Assertions; +import com.google.android.exoplayer2.util.EventDispatcher; import com.google.android.exoplayer2.util.Util; import java.io.IOException; import java.util.ArrayList; @@ -53,22 +55,21 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSourcesPublic; + @Nullable private Handler playbackThreadHandler; - // Accessed on the playback thread. + // Accessed on the playback thread only. private final List mediaSourceHolders; private final Map mediaSourceByMediaPeriod; private final Map mediaSourceByUid; - private final List pendingOnCompletionActions; private final boolean isAtomic; private final boolean useLazyPreparation; private final Timeline.Window window; private final Timeline.Period period; - @Nullable private Handler playbackThreadHandler; - @Nullable private Handler applicationThreadHandler; private boolean listenerNotificationScheduled; + private EventDispatcher pendingOnCompletionActions; private ShuffleOrder shuffleOrder; private int windowCount; private int periodCount; @@ -127,7 +128,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(); this.mediaSourcesPublic = new ArrayList<>(); this.mediaSourceHolders = new ArrayList<>(); - this.pendingOnCompletionActions = new ArrayList<>(); + this.pendingOnCompletionActions = new EventDispatcher<>(); this.isAtomic = isAtomic; this.useLazyPreparation = useLazyPreparation; window = new Timeline.Window(); @@ -141,19 +142,20 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources) { - addMediaSources(mediaSourcesPublic.size(), mediaSources, null); + addPublicMediaSources( + mediaSourcesPublic.size(), + mediaSources, + /* handler= */ null, + /* actionOnCompletion= */ null); } /** @@ -197,12 +209,13 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, @Nullable Runnable actionOnCompletion) { - addMediaSources(mediaSourcesPublic.size(), mediaSources, actionOnCompletion); + Collection mediaSources, Handler handler, Runnable actionOnCompletion) { + addPublicMediaSources(mediaSourcesPublic.size(), mediaSources, handler, actionOnCompletion); } /** @@ -214,7 +227,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources) { - addMediaSources(index, mediaSources, null); + addPublicMediaSources(index, mediaSources, /* handler= */ null, /* actionOnCompletion= */ null); } /** @@ -224,26 +237,16 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, @Nullable Runnable actionOnCompletion) { - for (MediaSource mediaSource : mediaSources) { - Assertions.checkNotNull(mediaSource); - } - List mediaSourceHolders = new ArrayList<>(mediaSources.size()); - for (MediaSource mediaSource : mediaSources) { - mediaSourceHolders.add(new MediaSourceHolder(mediaSource)); - } - mediaSourcesPublic.addAll(index, mediaSourceHolders); - if (playbackThreadHandler != null && !mediaSources.isEmpty()) { - playbackThreadHandler - .obtainMessage(MSG_ADD, new MessageData<>(index, mediaSourceHolders, actionOnCompletion)) - .sendToTarget(); - } else if (actionOnCompletion != null) { - actionOnCompletion.run(); - } + int index, + Collection mediaSources, + Handler handler, + Runnable actionOnCompletion) { + addPublicMediaSources(index, mediaSources, handler, actionOnCompletion); } /** @@ -259,26 +262,27 @@ public class ConcatenatingMediaSource extends CompositeMediaSourceNote: If you want to move the instance, it's preferable to use {@link #moveMediaSource(int, - * int, Runnable)} instead. + * int, Handler, Runnable)} instead. * *

Note: If you want to remove a set of contiguous sources, it's preferable to use {@link - * #removeMediaSourceRange(int, int, Runnable)} instead. + * #removeMediaSourceRange(int, int, Handler, Runnable)} instead. * * @param index The index at which the media source will be removed. This index must be in the * range of 0 <= index < {@link #getSize()}. + * @param handler The {@link Handler} to run {@code actionOnCompletion}. * @param actionOnCompletion A {@link Runnable} which is executed immediately after the media * source has been removed from the playlist. */ public final synchronized void removeMediaSource( - int index, @Nullable Runnable actionOnCompletion) { - removeMediaSourceRange(index, index + 1, actionOnCompletion); + int index, Handler handler, Runnable actionOnCompletion) { + removePublicMediaSources(index, index + 1, handler, actionOnCompletion); } /** @@ -296,7 +300,8 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(fromIndex, toIndex, actionOnCompletion)) - .sendToTarget(); - } else if (actionOnCompletion != null) { - actionOnCompletion.run(); - } + int fromIndex, int toIndex, Handler handler, Runnable actionOnCompletion) { + removePublicMediaSources(fromIndex, toIndex, handler, actionOnCompletion); } /** @@ -342,7 +335,8 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(currentIndex, newIndex, actionOnCompletion)) - .sendToTarget(); - } else if (actionOnCompletion != null) { - actionOnCompletion.run(); - } + int currentIndex, int newIndex, Handler handler, Runnable actionOnCompletion) { + movePublicMediaSource(currentIndex, newIndex, handler, actionOnCompletion); } /** Clears the playlist. */ public final synchronized void clear() { - clear(/* actionOnCompletion= */ null); + removeMediaSourceRange(0, getSize()); } /** * Clears the playlist and executes a custom action on completion. * + * @param handler The {@link Handler} to run {@code actionOnCompletion}. * @param actionOnCompletion A {@link Runnable} which is executed immediately after the playlist * has been cleared. */ - public final synchronized void clear(@Nullable Runnable actionOnCompletion) { - removeMediaSourceRange(0, getSize(), actionOnCompletion); + public final synchronized void clear(Handler handler, Runnable actionOnCompletion) { + removeMediaSourceRange(0, getSize(), handler, actionOnCompletion); } /** Returns the number of media sources in the playlist. */ @@ -410,41 +393,24 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(/* index= */ 0, shuffleOrder, actionOnCompletion)) - .sendToTarget(); - } else { - this.shuffleOrder = - shuffleOrder.getLength() > 0 ? shuffleOrder.cloneAndClear() : shuffleOrder; - if (actionOnCompletion != null) { - actionOnCompletion.run(); - } - } + ShuffleOrder shuffleOrder, Handler handler, Runnable actionOnCompletion) { + setPublicShuffleOrder(shuffleOrder, handler, actionOnCompletion); } + // CompositeMediaSource implementation. + @Override @Nullable public Object getTag() { @@ -458,13 +424,12 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, + @Nullable Handler handler, + @Nullable Runnable actionOnCompletion) { + Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + for (MediaSource mediaSource : mediaSources) { + Assertions.checkNotNull(mediaSource); + } + List mediaSourceHolders = new ArrayList<>(mediaSources.size()); + for (MediaSource mediaSource : mediaSources) { + mediaSourceHolders.add(new MediaSourceHolder(mediaSource)); + } + mediaSourcesPublic.addAll(index, mediaSourceHolders); + if (playbackThreadHandler != null && !mediaSources.isEmpty()) { + playbackThreadHandler + .obtainMessage( + MSG_ADD, new MessageData<>(index, mediaSourceHolders, handler, actionOnCompletion)) + .sendToTarget(); + } else if (actionOnCompletion != null && handler != null) { + handler.post(actionOnCompletion); + } + } + + @GuardedBy("this") + private void removePublicMediaSources( + int fromIndex, + int toIndex, + @Nullable Handler handler, + @Nullable Runnable actionOnCompletion) { + Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + Util.removeRange(mediaSourcesPublic, fromIndex, toIndex); + if (playbackThreadHandler != null) { + playbackThreadHandler + .obtainMessage( + MSG_REMOVE, new MessageData<>(fromIndex, toIndex, handler, actionOnCompletion)) + .sendToTarget(); + } else if (actionOnCompletion != null && handler != null) { + handler.post(actionOnCompletion); + } + } + + @GuardedBy("this") + private void movePublicMediaSource( + int currentIndex, + int newIndex, + @Nullable Handler handler, + @Nullable Runnable actionOnCompletion) { + Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + mediaSourcesPublic.add(newIndex, mediaSourcesPublic.remove(currentIndex)); + if (playbackThreadHandler != null) { + playbackThreadHandler + .obtainMessage( + MSG_MOVE, new MessageData<>(currentIndex, newIndex, handler, actionOnCompletion)) + .sendToTarget(); + } else if (actionOnCompletion != null && handler != null) { + handler.post(actionOnCompletion); + } + } + + @GuardedBy("this") + private void setPublicShuffleOrder( + ShuffleOrder shuffleOrder, @Nullable Handler handler, @Nullable Runnable actionOnCompletion) { + Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + Handler playbackThreadHandler = this.playbackThreadHandler; + if (playbackThreadHandler != null) { + int size = getSize(); + if (shuffleOrder.getLength() != size) { + shuffleOrder = + shuffleOrder + .cloneAndClear() + .cloneAndInsert(/* insertionIndex= */ 0, /* insertionCount= */ size); + } + playbackThreadHandler + .obtainMessage( + MSG_SET_SHUFFLE_ORDER, + new MessageData<>(/* index= */ 0, shuffleOrder, handler, actionOnCompletion)) + .sendToTarget(); + } else { + this.shuffleOrder = + shuffleOrder.getLength() > 0 ? shuffleOrder.cloneAndClear() : shuffleOrder; + if (actionOnCompletion != null && handler != null) { + handler.post(actionOnCompletion); + } + } + } + + // Internal methods. Called on the playback thread. + @SuppressWarnings("unchecked") private boolean handleMessage(Message msg) { if (playbackThreadHandler == null) { @@ -562,7 +618,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource>) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrder.cloneAndInsert(addMessage.index, addMessage.customData.size()); addMediaSourcesInternal(addMessage.index, addMessage.customData); - scheduleListenerNotification(addMessage.actionOnCompletion); + scheduleListenerNotification(addMessage.handler, addMessage.actionOnCompletion); break; case MSG_REMOVE: MessageData removeMessage = (MessageData) Util.castNonNull(msg.obj); @@ -576,30 +632,29 @@ public class ConcatenatingMediaSource extends CompositeMediaSource= fromIndex; index--) { removeMediaSourceInternal(index); } - scheduleListenerNotification(removeMessage.actionOnCompletion); + scheduleListenerNotification(removeMessage.handler, removeMessage.actionOnCompletion); break; case MSG_MOVE: MessageData moveMessage = (MessageData) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrder.cloneAndRemove(moveMessage.index, moveMessage.index + 1); shuffleOrder = shuffleOrder.cloneAndInsert(moveMessage.customData, 1); moveMediaSourceInternal(moveMessage.index, moveMessage.customData); - scheduleListenerNotification(moveMessage.actionOnCompletion); + scheduleListenerNotification(moveMessage.handler, moveMessage.actionOnCompletion); break; case MSG_SET_SHUFFLE_ORDER: MessageData shuffleOrderMessage = (MessageData) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrderMessage.customData; - scheduleListenerNotification(shuffleOrderMessage.actionOnCompletion); + scheduleListenerNotification( + shuffleOrderMessage.handler, shuffleOrderMessage.actionOnCompletion); break; case MSG_NOTIFY_LISTENER: notifyListener(); break; case MSG_ON_COMPLETION: - List actionsOnCompletion = (List) Util.castNonNull(msg.obj); - Handler handler = Assertions.checkNotNull(applicationThreadHandler); - for (int i = 0; i < actionsOnCompletion.size(); i++) { - handler.post(actionsOnCompletion.get(i)); - } + EventDispatcher actionsOnCompletion = + (EventDispatcher) Util.castNonNull(msg.obj); + actionsOnCompletion.dispatch(Runnable::run); break; default: throw new IllegalStateException(); @@ -607,34 +662,34 @@ public class ConcatenatingMediaSource extends CompositeMediaSource actionsOnCompletion = - pendingOnCompletionActions.isEmpty() - ? Collections.emptyList() - : new ArrayList<>(pendingOnCompletionActions); - pendingOnCompletionActions.clear(); + EventDispatcher actionsOnCompletion = pendingOnCompletionActions; + pendingOnCompletionActions = new EventDispatcher<>(); refreshSourceInfo( new ConcatenatedTimeline( mediaSourceHolders, windowCount, periodCount, shuffleOrder, isAtomic), /* manifest= */ null); - if (!actionsOnCompletion.isEmpty()) { - Assertions.checkNotNull(playbackThreadHandler) - .obtainMessage(MSG_ON_COMPLETION, actionsOnCompletion) - .sendToTarget(); - } + Assertions.checkNotNull(playbackThreadHandler) + .obtainMessage(MSG_ON_COMPLETION, actionsOnCompletion) + .sendToTarget(); } private void addMediaSourcesInternal( @@ -733,7 +788,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSource.addMediaSource(createFakeMediaSource(), timelineGrabber)); + () -> + mediaSource.addMediaSource(createFakeMediaSource(), new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(1); } finally { @@ -495,6 +499,7 @@ public final class ConcatenatingMediaSourceTest { mediaSource.addMediaSources( Arrays.asList( new MediaSource[] {createFakeMediaSource(), createFakeMediaSource()}), + new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); @@ -511,7 +516,8 @@ public final class ConcatenatingMediaSourceTest { final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); dummyMainThread.runOnMainThread( () -> - mediaSource.addMediaSource(/* index */ 0, createFakeMediaSource(), timelineGrabber)); + mediaSource.addMediaSource( + /* index */ 0, createFakeMediaSource(), new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(1); } finally { @@ -531,6 +537,7 @@ public final class ConcatenatingMediaSourceTest { /* index */ 0, Arrays.asList( new MediaSource[] {createFakeMediaSource(), createFakeMediaSource()}), + new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); @@ -549,7 +556,7 @@ public final class ConcatenatingMediaSourceTest { final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); dummyMainThread.runOnMainThread( - () -> mediaSource.removeMediaSource(/* index */ 0, timelineGrabber)); + () -> mediaSource.removeMediaSource(/* index */ 0, new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(0); } finally { @@ -571,7 +578,9 @@ public final class ConcatenatingMediaSourceTest { final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); dummyMainThread.runOnMainThread( - () -> mediaSource.moveMediaSource(/* fromIndex */ 1, /* toIndex */ 0, timelineGrabber)); + () -> + mediaSource.moveMediaSource( + /* fromIndex */ 1, /* toIndex */ 0, new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getWindowCount()).isEqualTo(2); } finally { @@ -819,7 +828,7 @@ public final class ConcatenatingMediaSourceTest { testRunner.prepareSource(); final TimelineGrabber timelineGrabber = new TimelineGrabber(testRunner); - dummyMainThread.runOnMainThread(() -> mediaSource.clear(timelineGrabber)); + dummyMainThread.runOnMainThread(() -> mediaSource.clear(new Handler(), timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.isEmpty()).isTrue(); @@ -965,7 +974,8 @@ public final class ConcatenatingMediaSourceTest { @Test public void testCustomCallbackBeforePreparationSetShuffleOrder() throws Exception { Runnable runnable = Mockito.mock(Runnable.class); - mediaSource.setShuffleOrder(new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 0), runnable); + mediaSource.setShuffleOrder( + new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 0), new Handler(), runnable); verify(runnable).run(); } @@ -981,7 +991,9 @@ public final class ConcatenatingMediaSourceTest { dummyMainThread.runOnMainThread( () -> mediaSource.setShuffleOrder( - new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 3), timelineGrabber)); + new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 3), + new Handler(), + timelineGrabber)); Timeline timeline = timelineGrabber.assertTimelineChangeBlocking(); assertThat(timeline.getFirstWindowIndex(/* shuffleModeEnabled= */ true)).isEqualTo(0); } finally { From 9dc5c87079dff00abe9c92854fd1f6b8f9e11a29 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 15 Jan 2019 11:07:05 +0000 Subject: [PATCH 19/23] Remove messages on release of ConcatenatingMediaSource. That was previously handled by the player. But since we switched to Handler messages instead of player messages, we should do that manually. PiperOrigin-RevId: 229341747 --- .../exoplayer2/source/ConcatenatingMediaSource.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java index fd567234e9..9874004ceb 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java @@ -478,10 +478,13 @@ public class ConcatenatingMediaSource extends CompositeMediaSource> addMessage = From 6330d46d9eb2d653503dd172df70ab34839ad6b5 Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 4 Feb 2019 17:40:01 +0000 Subject: [PATCH 20/23] Improve housekeeping of ConcatenatingMediaSource callbacks. When calling releaseSource(), all pending messages will be removed. That means that all action-on-completion callbacks which are somewhere in flight are just dropped without being called. This change adds code to keep track of the current state of each callback to allow all of them being called when the source is released. Issue:#5464 PiperOrigin-RevId: 232312528 --- RELEASENOTES.md | 2 + .../source/ConcatenatingMediaSource.java | 266 +++++++++++------- .../source/ConcatenatingMediaSourceTest.java | 40 ++- 3 files changed, 192 insertions(+), 116 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 271f2d94b9..5ba3aa7c8d 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -14,6 +14,8 @@ * OkHttp extension: Upgrade OkHttp dependency to 3.12.1. * MP3: Wider fix for issue where streams would play twice on some Samsung devices ([#4519](https://github.com/google/ExoPlayer/issues/4519)). +* Fix issue with dropped messages when releasing a `ConcatenatingMediaSource` + ([#5464](https://github.com/google/ExoPlayer/issues/5464)). ### 2.9.4 ### diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java index 9874004ceb..7baea9979f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java @@ -29,7 +29,6 @@ import com.google.android.exoplayer2.source.ShuffleOrder.DefaultShuffleOrder; import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.util.Assertions; -import com.google.android.exoplayer2.util.EventDispatcher; import com.google.android.exoplayer2.util.Util; import java.io.IOException; import java.util.ArrayList; @@ -37,9 +36,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.IdentityHashMap; import java.util.List; import java.util.Map; +import java.util.Set; /** * Concatenates multiple {@link MediaSource}s. The list of {@link MediaSource}s can be modified @@ -52,12 +53,19 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSourcesPublic; - @Nullable private Handler playbackThreadHandler; + + @GuardedBy("this") + private final Set pendingOnCompletionActions; + + @GuardedBy("this") + @Nullable + private Handler playbackThreadHandler; // Accessed on the playback thread only. private final List mediaSourceHolders; @@ -68,8 +76,8 @@ public class ConcatenatingMediaSource extends CompositeMediaSource pendingOnCompletionActions; + private boolean timelineUpdateScheduled; + private Set nextTimelineUpdateOnCompletionActions; private ShuffleOrder shuffleOrder; private int windowCount; private int periodCount; @@ -128,7 +136,8 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(); this.mediaSourcesPublic = new ArrayList<>(); this.mediaSourceHolders = new ArrayList<>(); - this.pendingOnCompletionActions = new EventDispatcher<>(); + this.nextTimelineUpdateOnCompletionActions = new HashSet<>(); + this.pendingOnCompletionActions = new HashSet<>(); this.isAtomic = isAtomic; this.useLazyPreparation = useLazyPreparation; window = new Timeline.Window(); @@ -149,13 +158,13 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, Handler handler, Runnable actionOnCompletion) { - addPublicMediaSources(mediaSourcesPublic.size(), mediaSources, handler, actionOnCompletion); + Collection mediaSources, Handler handler, Runnable onCompletionAction) { + addPublicMediaSources(mediaSourcesPublic.size(), mediaSources, handler, onCompletionAction); } /** @@ -227,7 +236,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources) { - addPublicMediaSources(index, mediaSources, /* handler= */ null, /* actionOnCompletion= */ null); + addPublicMediaSources(index, mediaSources, /* handler= */ null, /* onCompletionAction= */ null); } /** @@ -237,16 +246,16 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, Handler handler, - Runnable actionOnCompletion) { - addPublicMediaSources(index, mediaSources, handler, actionOnCompletion); + Runnable onCompletionAction) { + addPublicMediaSources(index, mediaSources, handler, onCompletionAction); } /** @@ -262,7 +271,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource mediaSources, @Nullable Handler handler, - @Nullable Runnable actionOnCompletion) { - Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + @Nullable Runnable onCompletionAction) { + Assertions.checkArgument((handler == null) == (onCompletionAction == null)); + Handler playbackThreadHandler = this.playbackThreadHandler; for (MediaSource mediaSource : mediaSources) { Assertions.checkNotNull(mediaSource); } @@ -535,12 +548,12 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(index, mediaSourceHolders, handler, actionOnCompletion)) + .obtainMessage(MSG_ADD, new MessageData<>(index, mediaSourceHolders, callbackAction)) .sendToTarget(); - } else if (actionOnCompletion != null && handler != null) { - handler.post(actionOnCompletion); + } else if (onCompletionAction != null && handler != null) { + handler.post(onCompletionAction); } } @@ -549,16 +562,17 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(fromIndex, toIndex, handler, actionOnCompletion)) + .obtainMessage(MSG_REMOVE, new MessageData<>(fromIndex, toIndex, callbackAction)) .sendToTarget(); - } else if (actionOnCompletion != null && handler != null) { - handler.post(actionOnCompletion); + } else if (onCompletionAction != null && handler != null) { + handler.post(onCompletionAction); } } @@ -567,23 +581,24 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(currentIndex, newIndex, handler, actionOnCompletion)) + .obtainMessage(MSG_MOVE, new MessageData<>(currentIndex, newIndex, callbackAction)) .sendToTarget(); - } else if (actionOnCompletion != null && handler != null) { - handler.post(actionOnCompletion); + } else if (onCompletionAction != null && handler != null) { + handler.post(onCompletionAction); } } @GuardedBy("this") private void setPublicShuffleOrder( - ShuffleOrder shuffleOrder, @Nullable Handler handler, @Nullable Runnable actionOnCompletion) { - Assertions.checkArgument((handler == null) == (actionOnCompletion == null)); + ShuffleOrder shuffleOrder, @Nullable Handler handler, @Nullable Runnable onCompletionAction) { + Assertions.checkArgument((handler == null) == (onCompletionAction == null)); Handler playbackThreadHandler = this.playbackThreadHandler; if (playbackThreadHandler != null) { int size = getSize(); @@ -593,20 +608,33 @@ public class ConcatenatingMediaSource extends CompositeMediaSource(/* index= */ 0, shuffleOrder, handler, actionOnCompletion)) + new MessageData<>(/* index= */ 0, shuffleOrder, callbackAction)) .sendToTarget(); } else { this.shuffleOrder = shuffleOrder.getLength() > 0 ? shuffleOrder.cloneAndClear() : shuffleOrder; - if (actionOnCompletion != null && handler != null) { - handler.post(actionOnCompletion); + if (onCompletionAction != null && handler != null) { + handler.post(onCompletionAction); } } } + @GuardedBy("this") + @Nullable + private HandlerAndRunnable createOnCompletionAction( + @Nullable Handler handler, @Nullable Runnable runnable) { + if (handler == null || runnable == null) { + return null; + } + HandlerAndRunnable handlerAndRunnable = new HandlerAndRunnable(handler, runnable); + pendingOnCompletionActions.add(handlerAndRunnable); + return handlerAndRunnable; + } + // Internal methods. Called on the playback thread. @SuppressWarnings("unchecked") @@ -617,7 +645,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource>) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrder.cloneAndInsert(addMessage.index, addMessage.customData.size()); addMediaSourcesInternal(addMessage.index, addMessage.customData); - scheduleListenerNotification(addMessage.handler, addMessage.actionOnCompletion); + scheduleTimelineUpdate(addMessage.onCompletionAction); break; case MSG_REMOVE: MessageData removeMessage = (MessageData) Util.castNonNull(msg.obj); @@ -631,29 +659,27 @@ public class ConcatenatingMediaSource extends CompositeMediaSource= fromIndex; index--) { removeMediaSourceInternal(index); } - scheduleListenerNotification(removeMessage.handler, removeMessage.actionOnCompletion); + scheduleTimelineUpdate(removeMessage.onCompletionAction); break; case MSG_MOVE: MessageData moveMessage = (MessageData) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrder.cloneAndRemove(moveMessage.index, moveMessage.index + 1); shuffleOrder = shuffleOrder.cloneAndInsert(moveMessage.customData, 1); moveMediaSourceInternal(moveMessage.index, moveMessage.customData); - scheduleListenerNotification(moveMessage.handler, moveMessage.actionOnCompletion); + scheduleTimelineUpdate(moveMessage.onCompletionAction); break; case MSG_SET_SHUFFLE_ORDER: MessageData shuffleOrderMessage = (MessageData) Util.castNonNull(msg.obj); shuffleOrder = shuffleOrderMessage.customData; - scheduleListenerNotification( - shuffleOrderMessage.handler, shuffleOrderMessage.actionOnCompletion); + scheduleTimelineUpdate(shuffleOrderMessage.onCompletionAction); break; - case MSG_NOTIFY_LISTENER: - notifyListener(); + case MSG_UPDATE_TIMELINE: + updateTimelineAndScheduleOnCompletionActions(); break; case MSG_ON_COMPLETION: - EventDispatcher actionsOnCompletion = - (EventDispatcher) Util.castNonNull(msg.obj); - actionsOnCompletion.dispatch(Runnable::run); + Set actions = (Set) Util.castNonNull(msg.obj); + dispatchOnCompletionActions(actions); break; default: throw new IllegalStateException(); @@ -661,36 +687,48 @@ public class ConcatenatingMediaSource extends CompositeMediaSource actionsOnCompletion = pendingOnCompletionActions; - pendingOnCompletionActions = new EventDispatcher<>(); + private void updateTimelineAndScheduleOnCompletionActions() { + timelineUpdateScheduled = false; + Set onCompletionActions = nextTimelineUpdateOnCompletionActions; + nextTimelineUpdateOnCompletionActions = new HashSet<>(); refreshSourceInfo( new ConcatenatedTimeline( mediaSourceHolders, windowCount, periodCount, shuffleOrder, isAtomic), /* manifest= */ null); - Assertions.checkNotNull(playbackThreadHandler) - .obtainMessage(MSG_ON_COMPLETION, actionsOnCompletion) + getPlaybackThreadHandlerOnPlaybackThread() + .obtainMessage(MSG_ON_COMPLETION, onCompletionActions) .sendToTarget(); } + @SuppressWarnings("GuardedBy") + private Handler getPlaybackThreadHandlerOnPlaybackThread() { + // Write access to this value happens on the playback thread only, so playback thread reads + // don't need to be synchronized. + return Assertions.checkNotNull(playbackThreadHandler); + } + + private synchronized void dispatchOnCompletionActions( + Set onCompletionActions) { + for (HandlerAndRunnable pendingAction : onCompletionActions) { + pendingAction.dispatch(); + } + pendingOnCompletionActions.removeAll(onCompletionActions); + } + private void addMediaSourcesInternal( int index, Collection mediaSourceHolders) { for (MediaSourceHolder mediaSourceHolder : mediaSourceHolders) { @@ -787,7 +825,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource { + SourceInfoRefreshListener listener = mock(SourceInfoRefreshListener.class); + mediaSource.addMediaSources(Arrays.asList(createMediaSources(2))); + mediaSource.prepareSource(listener, /* mediaTransferListener= */ null); + mediaSource.moveMediaSource( + /* currentIndex= */ 0, + /* newIndex= */ 1, + new Handler(), + callbackCalledCondition::open); + mediaSource.releaseSource(listener); + }); + assertThat(callbackCalledCondition.block(MediaSourceTestRunner.TIMEOUT_MS)).isTrue(); + } finally { + dummyMainThread.release(); + } + } + @Test public void testPeriodCreationWithAds() throws IOException, InterruptedException { // Create concatenated media source with ad child source. @@ -973,7 +997,7 @@ public final class ConcatenatingMediaSourceTest { @Test public void testCustomCallbackBeforePreparationSetShuffleOrder() throws Exception { - Runnable runnable = Mockito.mock(Runnable.class); + Runnable runnable = mock(Runnable.class); mediaSource.setShuffleOrder( new ShuffleOrder.UnshuffledShuffleOrder(/* length= */ 0), new Handler(), runnable); From 9ea62f70cdbd5e4d7b5afd11ad452bee74ca7f05 Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Mon, 4 Feb 2019 19:59:52 +0000 Subject: [PATCH 21/23] Update release notes --- RELEASENOTES.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5ba3aa7c8d..2490b99752 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,9 +2,11 @@ ### 2.9.5 ### -* Add `Handler` parameter to `ConcatenatingMediaSource` methods which take a - callback `Runnable`. * HLS: Parse `CHANNELS` attribute from `EXT-X-MEDIA` tag. +* ConcatenatingMediaSource: + * Add `Handler` parameter to methods that take a callback `Runnable`. + * Fix issue with dropped messages when releasing the source + ([#5464](https://github.com/google/ExoPlayer/issues/5464)). * ExtractorMediaSource: Fix issue that could cause the player to get stuck buffering at the end of the media. * PlayerView: Fix issue preventing `OnClickListener` from receiving events @@ -14,8 +16,6 @@ * OkHttp extension: Upgrade OkHttp dependency to 3.12.1. * MP3: Wider fix for issue where streams would play twice on some Samsung devices ([#4519](https://github.com/google/ExoPlayer/issues/4519)). -* Fix issue with dropped messages when releasing a `ConcatenatingMediaSource` - ([#5464](https://github.com/google/ExoPlayer/issues/5464)). ### 2.9.4 ### From c38cd1cca9e9deb50724d82fcbc3ac02c49505a9 Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Mon, 4 Feb 2019 20:02:43 +0000 Subject: [PATCH 22/23] Update release notes --- RELEASENOTES.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 2490b99752..b727e4ab49 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1177,7 +1177,7 @@ [here](https://medium.com/google-exoplayer/customizing-exoplayers-ui-components-728cf55ee07a#.9ewjg7avi). * Robustness improvements when handling MediaSource timeline changes and MediaPeriod transitions. -* EIA608: Support for caption styling and positioning. +* CEA-608: Support for caption styling and positioning. * MPEG-TS: Improved support: * Support injection of custom TS payload readers. * Support injection of custom section payload readers. @@ -1421,8 +1421,8 @@ V2 release. (#801). * MP3: Fix playback of some streams when stream length is unknown. * ID3: Support multiple frames of the same type in a single tag. -* EIA608: Correctly handle repeated control characters, fixing an issue in which - captions would immediately disappear. +* CEA-608: Correctly handle repeated control characters, fixing an issue in + which captions would immediately disappear. * AVC3: Fix decoder failures on some MediaTek devices in the case where the first buffer fed to the decoder does not start with SPS/PPS NAL units. * Misc bug fixes. From 4d843da6b0c4b98409e35d8d915a6ad7b15a2ddf Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Sun, 10 Feb 2019 19:06:35 +0000 Subject: [PATCH 23/23] Remove failing test on 2.9.5 --- .../source/ConcatenatingMediaSourceTest.java | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/library/core/src/test/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java b/library/core/src/test/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java index 2665b61f8a..9ec5078faf 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java @@ -589,29 +589,6 @@ public final class ConcatenatingMediaSourceTest { } } - @Test - public void testCustomCallbackIsCalledAfterRelease() throws IOException { - DummyMainThread dummyMainThread = new DummyMainThread(); - ConditionVariable callbackCalledCondition = new ConditionVariable(); - try { - dummyMainThread.runOnMainThread( - () -> { - SourceInfoRefreshListener listener = mock(SourceInfoRefreshListener.class); - mediaSource.addMediaSources(Arrays.asList(createMediaSources(2))); - mediaSource.prepareSource(listener, /* mediaTransferListener= */ null); - mediaSource.moveMediaSource( - /* currentIndex= */ 0, - /* newIndex= */ 1, - new Handler(), - callbackCalledCondition::open); - mediaSource.releaseSource(listener); - }); - assertThat(callbackCalledCondition.block(MediaSourceTestRunner.TIMEOUT_MS)).isTrue(); - } finally { - dummyMainThread.release(); - } - } - @Test public void testPeriodCreationWithAds() throws IOException, InterruptedException { // Create concatenated media source with ad child source.