From 754eb1527af9ff4211b7e164d2af6b46530f328c Mon Sep 17 00:00:00 2001 From: ibaker Date: Wed, 27 Apr 2022 13:10:36 +0100 Subject: [PATCH] Clean up `Util.inferContentType` methods This fixes some small niggles: 1. `inferContentType(String)` is documented to take a path, but in the tests we're passing full URIs. 2. A `String` parameter is usually a path, but also a MIME type or an extension. In the new methods, the meaning of a `String` parameter is always clear from the name of the method. 3. `inferContentType(String)` is always passed an extension in 'production' code (which has to be manually prefixed with a dot). 4. `inferContentType(Uri, @Nullable String)` always ignores the Uri if the String is non-null. IMO this logic is clearer to a reader if it's just in-lined at the call-site. These methods are used from the demo apps, so will be part of the stable API. PiperOrigin-RevId: 444826053 --- .../exoplayer2/gldemo/MainActivity.java | 8 +- .../demo/SampleChooserActivity.java | 6 +- .../exoplayer2/surfacedemo/MainActivity.java | 8 +- .../java/com/google/android/exoplayer2/C.java | 13 +-- .../google/android/exoplayer2/util/Util.java | 70 +++++++++----- .../android/exoplayer2/util/UtilTest.java | 91 +++++++++++++++---- 6 files changed, 141 insertions(+), 55 deletions(-) diff --git a/demos/gl/src/main/java/com/google/android/exoplayer2/gldemo/MainActivity.java b/demos/gl/src/main/java/com/google/android/exoplayer2/gldemo/MainActivity.java index 66f93ba03e..e1b339efcb 100644 --- a/demos/gl/src/main/java/com/google/android/exoplayer2/gldemo/MainActivity.java +++ b/demos/gl/src/main/java/com/google/android/exoplayer2/gldemo/MainActivity.java @@ -20,6 +20,7 @@ import android.content.Context; import android.content.Intent; import android.net.Uri; import android.os.Bundle; +import android.text.TextUtils; import android.widget.FrameLayout; import android.widget.Toast; import androidx.annotation.Nullable; @@ -156,7 +157,12 @@ public final class MainActivity extends Activity { DataSource.Factory dataSourceFactory = new DefaultDataSource.Factory(this); MediaSource mediaSource; - @C.ContentType int type = Util.inferContentType(uri, intent.getStringExtra(EXTENSION_EXTRA)); + @Nullable String fileExtension = intent.getStringExtra(EXTENSION_EXTRA); + @C.ContentType + int type = + TextUtils.isEmpty(fileExtension) + ? Util.inferContentType(uri) + : Util.inferContentTypeForExtension(fileExtension); if (type == C.TYPE_DASH) { mediaSource = new DashMediaSource.Factory(dataSourceFactory) diff --git a/demos/main/src/main/java/com/google/android/exoplayer2/demo/SampleChooserActivity.java b/demos/main/src/main/java/com/google/android/exoplayer2/demo/SampleChooserActivity.java index b0e37927d7..44f20f0e04 100644 --- a/demos/main/src/main/java/com/google/android/exoplayer2/demo/SampleChooserActivity.java +++ b/demos/main/src/main/java/com/google/android/exoplayer2/demo/SampleChooserActivity.java @@ -27,6 +27,7 @@ import android.content.res.AssetManager; import android.net.Uri; import android.os.AsyncTask; import android.os.Bundle; +import android.text.TextUtils; import android.util.JsonReader; import android.view.Menu; import android.view.MenuInflater; @@ -440,7 +441,10 @@ public class SampleChooserActivity extends AppCompatActivity } else { @Nullable String adaptiveMimeType = - Util.getAdaptiveMimeTypeForContentType(Util.inferContentType(uri, extension)); + Util.getAdaptiveMimeTypeForContentType( + TextUtils.isEmpty(extension) + ? Util.inferContentType(uri) + : Util.inferContentTypeForExtension(extension)); mediaItem .setUri(uri) .setMediaMetadata(new MediaMetadata.Builder().setTitle(title).build()) diff --git a/demos/surface/src/main/java/com/google/android/exoplayer2/surfacedemo/MainActivity.java b/demos/surface/src/main/java/com/google/android/exoplayer2/surfacedemo/MainActivity.java index 4cfea99682..b83522d51d 100644 --- a/demos/surface/src/main/java/com/google/android/exoplayer2/surfacedemo/MainActivity.java +++ b/demos/surface/src/main/java/com/google/android/exoplayer2/surfacedemo/MainActivity.java @@ -19,6 +19,7 @@ import android.app.Activity; import android.content.Intent; import android.net.Uri; import android.os.Bundle; +import android.text.TextUtils; import android.view.Surface; import android.view.SurfaceControl; import android.view.SurfaceHolder; @@ -201,7 +202,12 @@ public final class MainActivity extends Activity { DataSource.Factory dataSourceFactory = new DefaultDataSource.Factory(this); MediaSource mediaSource; - @C.ContentType int type = Util.inferContentType(uri, intent.getStringExtra(EXTENSION_EXTRA)); + @Nullable String fileExtension = intent.getStringExtra(EXTENSION_EXTRA); + @C.ContentType + int type = + TextUtils.isEmpty(fileExtension) + ? Util.inferContentType(uri) + : Util.inferContentTypeForExtension(fileExtension); if (type == C.TYPE_DASH) { mediaSource = new DashMediaSource.Factory(dataSourceFactory) diff --git a/library/common/src/main/java/com/google/android/exoplayer2/C.java b/library/common/src/main/java/com/google/android/exoplayer2/C.java index e68830f60d..113f7644cc 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/C.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/C.java @@ -722,18 +722,15 @@ public final class C { @Target({FIELD, METHOD, PARAMETER, LOCAL_VARIABLE, TYPE_USE}) @IntDef({TYPE_DASH, TYPE_SS, TYPE_HLS, TYPE_RTSP, TYPE_OTHER}) public @interface ContentType {} - /** Value returned by {@link Util#inferContentType} for DASH manifests. */ + /** Value representing a DASH manifest. */ public static final int TYPE_DASH = 0; - /** Value returned by {@link Util#inferContentType} for Smooth Streaming manifests. */ + /** Value representing a Smooth Streaming manifest. */ public static final int TYPE_SS = 1; - /** Value returned by {@link Util#inferContentType} for HLS manifests. */ + /** Value representing an HLS manifest. */ public static final int TYPE_HLS = 2; - /** Value returned by {@link Util#inferContentType} for RTSP. */ + /** Value representing an RTSP stream. */ public static final int TYPE_RTSP = 3; - /** - * Value returned by {@link Util#inferContentType} for files other than DASH, HLS or Smooth - * Streaming manifests, or RTSP URIs. - */ + /** Value representing files other than DASH, HLS or Smooth Streaming manifests, or RTSP URIs. */ public static final int TYPE_OTHER = 4; /** A return value for methods where the end of an input was encountered. */ diff --git a/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java b/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java index 6f33ef788e..962fd48233 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java @@ -156,8 +156,9 @@ public final class Util { + "(T(([0-9]*)H)?(([0-9]*)M)?(([0-9.]*)S)?)?$"); private static final Pattern ESCAPED_CHARACTER_PATTERN = Pattern.compile("%([A-Fa-f0-9]{2})"); - // https://docs.microsoft.com/en-us/azure/media-services/previous/media-services-deliver-content-overview#URLs. - private static final Pattern ISM_URL_PATTERN = Pattern.compile(".*\\.isml?(?:/(manifest(.*))?)?"); + // https://docs.microsoft.com/en-us/azure/media-services/previous/media-services-deliver-content-overview#URLs + private static final Pattern ISM_PATH_PATTERN = + Pattern.compile(".*\\.isml?(?:/(manifest(.*))?)?", Pattern.CASE_INSENSITIVE); private static final String ISM_HLS_FORMAT_EXTENSION = "format=m3u8-aapl"; private static final String ISM_DASH_FORMAT_EXTENSION = "format=mpd-time-csf"; @@ -1818,16 +1819,14 @@ public final class Util { } /** - * Makes a best guess to infer the {@link ContentType} from a {@link Uri}. - * - * @param uri The {@link Uri}. - * @param overrideExtension If not null, used to infer the type. - * @return The content type. + * @deprecated Use {@link #inferContentTypeForExtension(String)} when {@code overrideExtension} is + * non-empty, and {@link #inferContentType(Uri)} otherwise. */ + @Deprecated public static @ContentType int inferContentType(Uri uri, @Nullable String overrideExtension) { return TextUtils.isEmpty(overrideExtension) ? inferContentType(uri) - : inferContentType("." + overrideExtension); + : inferContentTypeForExtension(overrideExtension); } /** @@ -1843,23 +1842,19 @@ public final class Util { } @Nullable String path = uri.getPath(); - return path == null ? C.TYPE_OTHER : inferContentType(path); - } - - /** - * Makes a best guess to infer the {@link ContentType} from a file name. - * - * @param fileName Name of the file. It can include the path of the file. - * @return The content type. - */ - public static @ContentType int inferContentType(String fileName) { - fileName = Ascii.toLowerCase(fileName); - if (fileName.endsWith(".mpd")) { - return C.TYPE_DASH; - } else if (fileName.endsWith(".m3u8")) { - return C.TYPE_HLS; + if (path == null) { + return C.TYPE_OTHER; } - Matcher ismMatcher = ISM_URL_PATTERN.matcher(fileName); + int lastDotIndex = path.lastIndexOf('.'); + if (lastDotIndex >= 0) { + @C.ContentType + int contentType = inferContentTypeForExtension(path.substring(lastDotIndex + 1)); + if (contentType != C.TYPE_OTHER) { + return contentType; + } + } + + Matcher ismMatcher = ISM_PATH_PATTERN.matcher(path); if (ismMatcher.matches()) { @Nullable String extensions = ismMatcher.group(2); if (extensions != null) { @@ -1871,6 +1866,31 @@ public final class Util { } return C.TYPE_SS; } + + return C.TYPE_OTHER; + } + + /** + * @deprecated Use {@link Uri#parse(String)} and {@link #inferContentType(Uri)} for full file + * paths or {@link #inferContentTypeForExtension(String)} for extensions. + */ + @Deprecated + public static @ContentType int inferContentType(String fileName) { + return inferContentType(Uri.parse("file:///" + fileName)); + } + + /** + * Makes a best guess to infer the {@link ContentType} from a file extension. + * + * @param fileExtension The extension of the file (excluding the '.'). + * @return The content type. + */ + public static @ContentType int inferContentTypeForExtension(String fileExtension) { + if (Ascii.equalsIgnoreCase("mpd", fileExtension)) { + return C.TYPE_DASH; + } else if (Ascii.equalsIgnoreCase("m3u8", fileExtension)) { + return C.TYPE_HLS; + } return C.TYPE_OTHER; } @@ -1933,7 +1953,7 @@ public final class Util { if (path == null) { return uri; } - Matcher ismMatcher = ISM_URL_PATTERN.matcher(Ascii.toLowerCase(path)); + Matcher ismMatcher = ISM_PATH_PATTERN.matcher(path); if (ismMatcher.matches() && ismMatcher.group(1) == null) { // Add missing "Manifest" suffix. return Uri.withAppendedPath(uri, "Manifest"); diff --git a/library/common/src/test/java/com/google/android/exoplayer2/util/UtilTest.java b/library/common/src/test/java/com/google/android/exoplayer2/util/UtilTest.java index d4ad925c0a..43b5bdb6d0 100644 --- a/library/common/src/test/java/com/google/android/exoplayer2/util/UtilTest.java +++ b/library/common/src/test/java/com/google/android/exoplayer2/util/UtilTest.java @@ -103,50 +103,103 @@ public class UtilTest { @Test public void inferContentType_handlesHlsIsmUris() { - assertThat(Util.inferContentType("http://a.b/c.ism/manifest(format=m3u8-aapl)")) + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.ism/manifest(format=m3u8-aapl)"))) .isEqualTo(C.TYPE_HLS); - assertThat(Util.inferContentType("http://a.b/c.ism/manifest(format=m3u8-aapl,quality=hd)")) + assertThat( + Util.inferContentType( + Uri.parse("http://a.b/c.ism/manifest(format=m3u8-aapl,quality=hd)"))) .isEqualTo(C.TYPE_HLS); - assertThat(Util.inferContentType("http://a.b/c.ism/manifest(quality=hd,format=m3u8-aapl)")) + assertThat( + Util.inferContentType( + Uri.parse("http://a.b/c.ism/manifest(quality=hd,format=m3u8-aapl)"))) .isEqualTo(C.TYPE_HLS); } @Test public void inferContentType_handlesHlsIsmV3Uris() { - assertThat(Util.inferContentType("http://a.b/c.ism/manifest(format=m3u8-aapl-v3)")) + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.ism/manifest(format=m3u8-aapl-v3)"))) .isEqualTo(C.TYPE_HLS); - assertThat(Util.inferContentType("http://a.b/c.ism/manifest(format=m3u8-aapl-v3,quality=hd)")) + assertThat( + Util.inferContentType( + Uri.parse("http://a.b/c.ism/manifest(format=m3u8-aapl-v3,quality=hd)"))) .isEqualTo(C.TYPE_HLS); - assertThat(Util.inferContentType("http://a.b/c.ism/manifest(quality=hd,format=m3u8-aapl-v3)")) + assertThat( + Util.inferContentType( + Uri.parse("http://a.b/c.ism/manifest(quality=hd,format=m3u8-aapl-v3)"))) .isEqualTo(C.TYPE_HLS); } @Test public void inferContentType_handlesDashIsmUris() { - assertThat(Util.inferContentType("http://a.b/c.isml/manifest(format=mpd-time-csf)")) + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.isml/manifest(format=mpd-time-csf)"))) .isEqualTo(C.TYPE_DASH); - assertThat(Util.inferContentType("http://a.b/c.isml/manifest(format=mpd-time-csf,quality=hd)")) + assertThat( + Util.inferContentType( + Uri.parse("http://a.b/c.isml/manifest(format=mpd-time-csf,quality=hd)"))) .isEqualTo(C.TYPE_DASH); - assertThat(Util.inferContentType("http://a.b/c.isml/manifest(quality=hd,format=mpd-time-csf)")) + assertThat( + Util.inferContentType( + Uri.parse("http://a.b/c.isml/manifest(quality=hd,format=mpd-time-csf)"))) .isEqualTo(C.TYPE_DASH); } @Test public void inferContentType_handlesSmoothStreamingIsmUris() { - assertThat(Util.inferContentType("http://a.b/c.ism")).isEqualTo(C.TYPE_SS); - assertThat(Util.inferContentType("http://a.b/c.isml")).isEqualTo(C.TYPE_SS); - assertThat(Util.inferContentType("http://a.b/c.ism/")).isEqualTo(C.TYPE_SS); - assertThat(Util.inferContentType("http://a.b/c.isml/")).isEqualTo(C.TYPE_SS); - assertThat(Util.inferContentType("http://a.b/c.ism/Manifest")).isEqualTo(C.TYPE_SS); - assertThat(Util.inferContentType("http://a.b/c.isml/manifest")).isEqualTo(C.TYPE_SS); - assertThat(Util.inferContentType("http://a.b/c.isml/manifest(filter=x)")).isEqualTo(C.TYPE_SS); - assertThat(Util.inferContentType("http://a.b/c.isml/manifest_hd")).isEqualTo(C.TYPE_SS); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.ism"))).isEqualTo(C.TYPE_SS); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.isml"))).isEqualTo(C.TYPE_SS); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.ism/"))).isEqualTo(C.TYPE_SS); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.isml/"))).isEqualTo(C.TYPE_SS); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.ism/Manifest"))).isEqualTo(C.TYPE_SS); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.isml/manifest"))).isEqualTo(C.TYPE_SS); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.isml/manifest(filter=x)"))) + .isEqualTo(C.TYPE_SS); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.isml/manifest_hd"))) + .isEqualTo(C.TYPE_SS); } @Test public void inferContentType_handlesOtherIsmUris() { - assertThat(Util.inferContentType("http://a.b/c.ism/video.mp4")).isEqualTo(C.TYPE_OTHER); - assertThat(Util.inferContentType("http://a.b/c.ism/prefix-manifest")).isEqualTo(C.TYPE_OTHER); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.ism/video.mp4"))) + .isEqualTo(C.TYPE_OTHER); + assertThat(Util.inferContentType(Uri.parse("http://a.b/c.ism/prefix-manifest"))) + .isEqualTo(C.TYPE_OTHER); + } + + /** + * Test that the deprecated {@link Util#inferContentType(String)} works when passed only a file + * extension and the leading dot. + */ + @SuppressWarnings("deprecation") + @Test + public void inferContentType_extensionAsPath() { + assertThat(Util.inferContentType(".m3u8")).isEqualTo(C.TYPE_HLS); + assertThat(Util.inferContentType(".mpd")).isEqualTo(C.TYPE_DASH); + assertThat(Util.inferContentType(".mp4")).isEqualTo(C.TYPE_OTHER); + } + + // Testing deprecated method. + @SuppressWarnings("deprecation") + @Test + public void inferContentType_extensionOverride() { + assertThat( + Util.inferContentType( + Uri.parse("file:///path/to/something.mpd"), /* overrideExtension= */ null)) + .isEqualTo(C.TYPE_DASH); + assertThat( + Util.inferContentType( + Uri.parse("file:///path/to/something.mpd"), /* overrideExtension= */ "")) + .isEqualTo(C.TYPE_DASH); + assertThat( + Util.inferContentType( + Uri.parse("file:///path/to/something.mpd"), /* overrideExtension= */ "m3u8")) + .isEqualTo(C.TYPE_HLS); + } + + @Test + public void inferContentTypeForExtension() { + assertThat(Util.inferContentTypeForExtension("m3u8")).isEqualTo(C.TYPE_HLS); + assertThat(Util.inferContentTypeForExtension("mpd")).isEqualTo(C.TYPE_DASH); + assertThat(Util.inferContentTypeForExtension("mp4")).isEqualTo(C.TYPE_OTHER); } @Test