[go: nahoru, domu]

[API Review] Remove OutputOptions.getType()

 The same functionality can be achieved with existing language features,
 specifically `instanceof` in Java. Replaces all switch statements using
 this method with if-conditions and removed the TYPE_ enumeration.

Bug: 198241716
Test: ./gradlew bOS
Change-Id: I7547788952a7b9f38258f5e9114b5815585423ed
diff --git a/camera/camera-video/src/androidTest/java/androidx/camera/video/OutputOptionsTest.kt b/camera/camera-video/src/androidTest/java/androidx/camera/video/OutputOptionsTest.kt
index 3b4791e..bf7508e 100644
--- a/camera/camera-video/src/androidTest/java/androidx/camera/video/OutputOptionsTest.kt
+++ b/camera/camera-video/src/androidTest/java/androidx/camera/video/OutputOptionsTest.kt
@@ -47,7 +47,6 @@
             .build()
 
         assertThat(fileOutputOptions).isNotNull()
-        assertThat(fileOutputOptions.type).isEqualTo(OutputOptions.OPTIONS_TYPE_FILE)
         assertThat(fileOutputOptions.file).isNotNull()
         assertThat(fileOutputOptions.fileSizeLimit).isEqualTo(FILE_SIZE_LIMIT)
         savedFile.delete()
@@ -74,7 +73,6 @@
             MediaStore.Video.Media.EXTERNAL_CONTENT_URI
         )
         assertThat(mediaStoreOutputOptions.contentValues).isEqualTo(contentValues)
-        assertThat(mediaStoreOutputOptions.type).isEqualTo(OutputOptions.OPTIONS_TYPE_MEDIA_STORE)
         assertThat(mediaStoreOutputOptions.fileSizeLimit).isEqualTo(FILE_SIZE_LIMIT)
     }
 
@@ -91,7 +89,6 @@
                 .build()
 
             assertThat(fdOutputOptions).isNotNull()
-            assertThat(fdOutputOptions.type).isEqualTo(OutputOptions.OPTIONS_TYPE_FILE_DESCRIPTOR)
             assertThat(fdOutputOptions.parcelFileDescriptor).isNotNull()
             assertThat(fdOutputOptions.fileSizeLimit).isEqualTo(FILE_SIZE_LIMIT)
         }
diff --git a/camera/camera-video/src/main/java/androidx/camera/video/FileDescriptorOutputOptions.java b/camera/camera-video/src/main/java/androidx/camera/video/FileDescriptorOutputOptions.java
index b293823..dff15f9 100644
--- a/camera/camera-video/src/main/java/androidx/camera/video/FileDescriptorOutputOptions.java
+++ b/camera/camera-video/src/main/java/androidx/camera/video/FileDescriptorOutputOptions.java
@@ -38,7 +38,6 @@
 
     FileDescriptorOutputOptions(
             @NonNull FileDescriptorOutputOptionsInternal fileDescriptorOutputOptionsInternal) {
-        super(OPTIONS_TYPE_FILE_DESCRIPTOR);
         Preconditions.checkNotNull(fileDescriptorOutputOptionsInternal,
                 "FileDescriptorOutputOptionsInternal can't be null.");
         mFileDescriptorOutputOptionsInternal = fileDescriptorOutputOptionsInternal;
diff --git a/camera/camera-video/src/main/java/androidx/camera/video/FileOutputOptions.java b/camera/camera-video/src/main/java/androidx/camera/video/FileOutputOptions.java
index 610f235..e8b111e 100644
--- a/camera/camera-video/src/main/java/androidx/camera/video/FileOutputOptions.java
+++ b/camera/camera-video/src/main/java/androidx/camera/video/FileOutputOptions.java
@@ -36,7 +36,6 @@
     private final FileOutputOptionsInternal mFileOutputOptionsInternal;
 
     FileOutputOptions(@NonNull FileOutputOptionsInternal fileOutputOptionsInternal) {
-        super(OPTIONS_TYPE_FILE);
         Preconditions.checkNotNull(fileOutputOptionsInternal,
                 "FileOutputOptionsInternal can't be null.");
         mFileOutputOptionsInternal = fileOutputOptionsInternal;
diff --git a/camera/camera-video/src/main/java/androidx/camera/video/MediaStoreOutputOptions.java b/camera/camera-video/src/main/java/androidx/camera/video/MediaStoreOutputOptions.java
index e2dda51..4edabe9 100644
--- a/camera/camera-video/src/main/java/androidx/camera/video/MediaStoreOutputOptions.java
+++ b/camera/camera-video/src/main/java/androidx/camera/video/MediaStoreOutputOptions.java
@@ -58,7 +58,6 @@
 
     MediaStoreOutputOptions(
             @NonNull MediaStoreOutputOptionsInternal mediaStoreOutputOptionsInternal) {
-        super(OPTIONS_TYPE_MEDIA_STORE);
         Preconditions.checkNotNull(mediaStoreOutputOptionsInternal,
                 "MediaStoreOutputOptionsInternal can't be null.");
         mMediaStoreOutputOptionsInternal = mediaStoreOutputOptionsInternal;
diff --git a/camera/camera-video/src/main/java/androidx/camera/video/OutputOptions.java b/camera/camera-video/src/main/java/androidx/camera/video/OutputOptions.java
index 30c1cd9..f98ca59 100644
--- a/camera/camera-video/src/main/java/androidx/camera/video/OutputOptions.java
+++ b/camera/camera-video/src/main/java/androidx/camera/video/OutputOptions.java
@@ -16,12 +16,7 @@
 
 package androidx.camera.video;
 
-import androidx.annotation.IntDef;
 import androidx.annotation.NonNull;
-import androidx.annotation.RestrictTo;
-
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
 
 /**
  * Options for configuring output destination.
@@ -31,40 +26,7 @@
     /** Represents an unbound file size. */
     public static final int FILE_SIZE_UNLIMITED = 0;
 
-    /** Output options of {@link FileOutputOptions}. */
-    public static final int OPTIONS_TYPE_FILE = 0;
-    /** Output options of {@link FileDescriptorOutputOptions}. */
-    public static final int OPTIONS_TYPE_FILE_DESCRIPTOR = 1;
-    /** Output options of {@link MediaStoreOutputOptions}. */
-    public static final int OPTIONS_TYPE_MEDIA_STORE = 2;
-
-    /** @hide */
-    @IntDef({OPTIONS_TYPE_FILE, OPTIONS_TYPE_FILE_DESCRIPTOR, OPTIONS_TYPE_MEDIA_STORE})
-    @Retention(RetentionPolicy.SOURCE)
-    @RestrictTo(RestrictTo.Scope.LIBRARY)
-    public @interface OptionsType {
-    }
-
-    @OptionsType
-    private final int mType;
-
-    OutputOptions(@OptionsType int type) {
-        mType = type;
-    }
-
-    /**
-     * Returns the subclass type of this output options.
-     *
-     * <p>Output options are limited to a distinct number of subclasses. Each subclass is
-     * represented by a type. The type can be used to determine which class cast the output
-     * options to in order to obtain more detailed information about the particular output
-     * destination.
-     *
-     * @return the type of this output options.
-     */
-    @OptionsType
-    public int getType() {
-        return mType;
+    OutputOptions() {
     }
 
     /**
diff --git a/camera/camera-video/src/main/java/androidx/camera/video/Recorder.java b/camera/camera-video/src/main/java/androidx/camera/video/Recorder.java
index 58a3ab4..16c568b 100644
--- a/camera/camera-video/src/main/java/androidx/camera/video/Recorder.java
+++ b/camera/camera-video/src/main/java/androidx/camera/video/Recorder.java
@@ -1295,75 +1295,62 @@
     private void setupMediaMuxer(@NonNull OutputOptions options) throws IOException {
         int muxerOutputFormat = MediaSpec.outputFormatToMuxerFormat(
                 getObservableData(mMediaSpec).getOutputFormat());
-        switch (options.getType()) {
-            case OutputOptions.OPTIONS_TYPE_FILE:
-                if (!(options instanceof FileOutputOptions)) {
-                    throw new AssertionError("Invalid OutputOptions type");
-                }
-                FileOutputOptions fileOutputOptions = (FileOutputOptions) options;
-                File file = fileOutputOptions.getFile();
-                mMediaMuxer = new MediaMuxer(file.getAbsolutePath(), muxerOutputFormat);
-                mOutputUri = Uri.fromFile(file);
-                break;
-            case OutputOptions.OPTIONS_TYPE_FILE_DESCRIPTOR:
-                if (!(options instanceof FileDescriptorOutputOptions)) {
-                    throw new AssertionError("Invalid OutputOptions type");
-                }
-                FileDescriptorOutputOptions fileDescriptorOutputOptions =
-                        (FileDescriptorOutputOptions) options;
-                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-                    mMediaMuxer = Api26Impl.createMediaMuxer(
-                            fileDescriptorOutputOptions.getParcelFileDescriptor()
-                                    .getFileDescriptor(), muxerOutputFormat);
-                } else {
-                    throw new IOException(
-                            "MediaMuxer doesn't accept FileDescriptor as output destination.");
-                }
-                break;
-            case OutputOptions.OPTIONS_TYPE_MEDIA_STORE:
-                if (!(options instanceof MediaStoreOutputOptions)) {
-                    throw new AssertionError("Invalid OutputOptions type");
-                }
-                MediaStoreOutputOptions mediaStoreOutputOptions = (MediaStoreOutputOptions) options;
+        if (options instanceof FileOutputOptions) {
+            FileOutputOptions fileOutputOptions = (FileOutputOptions) options;
+            File file = fileOutputOptions.getFile();
+            mMediaMuxer = new MediaMuxer(file.getAbsolutePath(), muxerOutputFormat);
+            mOutputUri = Uri.fromFile(file);
+        } else if (options instanceof FileDescriptorOutputOptions) {
+            FileDescriptorOutputOptions fileDescriptorOutputOptions =
+                    (FileDescriptorOutputOptions) options;
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+                mMediaMuxer = Api26Impl.createMediaMuxer(
+                        fileDescriptorOutputOptions.getParcelFileDescriptor()
+                                .getFileDescriptor(), muxerOutputFormat);
+            } else {
+                throw new IOException(
+                        "MediaMuxer doesn't accept FileDescriptor as output destination.");
+            }
+        } else if (options instanceof MediaStoreOutputOptions) {
+            MediaStoreOutputOptions mediaStoreOutputOptions = (MediaStoreOutputOptions) options;
 
-                ContentValues contentValues =
-                        new ContentValues(mediaStoreOutputOptions.getContentValues());
-                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
-                    // Toggle on pending status for the video file.
-                    contentValues.put(MediaStore.Video.Media.IS_PENDING, PENDING);
-                }
-                Uri outputUri = mediaStoreOutputOptions.getContentResolver().insert(
-                        mediaStoreOutputOptions.getCollection(), contentValues);
-                if (outputUri == null) {
-                    throw new IOException("Unable to create MediaStore entry.");
-                }
-                mOutputUri = outputUri;  // Guarantee mOutputUri is non-null value.
+            ContentValues contentValues =
+                    new ContentValues(mediaStoreOutputOptions.getContentValues());
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
+                // Toggle on pending status for the video file.
+                contentValues.put(MediaStore.Video.Media.IS_PENDING, PENDING);
+            }
+            Uri outputUri = mediaStoreOutputOptions.getContentResolver().insert(
+                    mediaStoreOutputOptions.getCollection(), contentValues);
+            if (outputUri == null) {
+                throw new IOException("Unable to create MediaStore entry.");
+            }
+            mOutputUri = outputUri;  // Guarantee mOutputUri is non-null value.
 
-                if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
-                    String path =
-                            OutputUtil.getAbsolutePathFromUri(
-                                    mediaStoreOutputOptions.getContentResolver(),
-                                    mOutputUri, MEDIA_COLUMN);
-                    if (path == null) {
-                        throw new IOException("Unable to get path from uri " + mOutputUri);
-                    }
-                    File parentFile = new File(path).getParentFile();
-                    if (parentFile != null && !parentFile.mkdirs()) {
-                        Logger.w(TAG, "Failed to create folder for " + path);
-                    }
-                    mMediaMuxer = new MediaMuxer(path, muxerOutputFormat);
-                } else {
-                    ParcelFileDescriptor fileDescriptor =
-                            mediaStoreOutputOptions.getContentResolver().openFileDescriptor(
-                                    mOutputUri, "rw");
-                    mMediaMuxer = Api26Impl.createMediaMuxer(fileDescriptor.getFileDescriptor(),
-                            muxerOutputFormat);
-                    fileDescriptor.close();
+            if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
+                String path =
+                        OutputUtil.getAbsolutePathFromUri(
+                                mediaStoreOutputOptions.getContentResolver(),
+                                mOutputUri, MEDIA_COLUMN);
+                if (path == null) {
+                    throw new IOException("Unable to get path from uri " + mOutputUri);
                 }
-                break;
-            default:
-                throw new AssertionError(
-                        "Invalid output options type." + options.getType());
+                File parentFile = new File(path).getParentFile();
+                if (parentFile != null && !parentFile.mkdirs()) {
+                    Logger.w(TAG, "Failed to create folder for " + path);
+                }
+                mMediaMuxer = new MediaMuxer(path, muxerOutputFormat);
+            } else {
+                ParcelFileDescriptor fileDescriptor =
+                        mediaStoreOutputOptions.getContentResolver().openFileDescriptor(
+                                mOutputUri, "rw");
+                mMediaMuxer = Api26Impl.createMediaMuxer(fileDescriptor.getFileDescriptor(),
+                        muxerOutputFormat);
+                fileDescriptor.close();
+            }
+        } else {
+            throw new AssertionError(
+                    "Invalid output options type: " + options.getClass().getSimpleName());
         }
         // TODO: Add more metadata to MediaMuxer, e.g. location information.
         if (mSurfaceTransformationInfo != null) {
@@ -1795,7 +1782,7 @@
         OutputOptions outputOptions = mInProgressRecording.getOutputOptions();
         RecordingStats stats = getInProgressRecordingStats();
 
-        if (outputOptions.getType() == OutputOptions.OPTIONS_TYPE_MEDIA_STORE) {
+        if (outputOptions instanceof MediaStoreOutputOptions) {
             // Toggle off pending status for the video file.
             finalizeMediaStoreFile((MediaStoreOutputOptions) outputOptions);
         }
diff --git a/camera/camera-video/src/main/java/androidx/camera/video/VideoRecordEvent.java b/camera/camera-video/src/main/java/androidx/camera/video/VideoRecordEvent.java
index 210b8e1..2811f91 100644
--- a/camera/camera-video/src/main/java/androidx/camera/video/VideoRecordEvent.java
+++ b/camera/camera-video/src/main/java/androidx/camera/video/VideoRecordEvent.java
@@ -260,9 +260,8 @@
          *
          * <p>This error is generated when invalid output options have been used while preparing a
          * recording, such as with the {@link Recorder#prepareRecording(MediaStoreOutputOptions)}
-         * method. The error will depend on the {@linkplain OutputOptions#getType() type} of options
-         * used, and more information about the error can be retrieved from
-         * {@link Finalize#getCause()}.
+         * method. The error will depend on the subclass of {@link OutputOptions} used, and more
+         * information about the error can be retrieved from {@link Finalize#getCause()}.
          */
         public static final int ERROR_INVALID_OUTPUT_OPTIONS = 5;
 
diff --git a/camera/integration-tests/coretestapp/src/main/java/androidx/camera/integration/core/CameraXActivity.java b/camera/integration-tests/coretestapp/src/main/java/androidx/camera/integration/core/CameraXActivity.java
index 25d0175..3c8a8f7a 100644
--- a/camera/integration-tests/coretestapp/src/main/java/androidx/camera/integration/core/CameraXActivity.java
+++ b/camera/integration-tests/coretestapp/src/main/java/androidx/camera/integration/core/CameraXActivity.java
@@ -487,31 +487,28 @@
                     case ERROR_INSUFFICIENT_DISK:
                     case ERROR_SOURCE_INACTIVE:
                         Uri uri = finalize.getOutputResults().getOutputUri();
+                        OutputOptions outputOptions = finalize.getOutputOptions();
                         String msg;
                         String videoFilePath;
-                        switch (finalize.getOutputOptions().getType()) {
-                            case OutputOptions.OPTIONS_TYPE_MEDIA_STORE:
-                                msg = "Saved uri " + uri;
-                                videoFilePath = getAbsolutePathFromUri(
+                        if (outputOptions instanceof MediaStoreOutputOptions) {
+                            msg = "Saved uri " + uri;
+                            videoFilePath = getAbsolutePathFromUri(
                                     getApplicationContext().getContentResolver(),
                                     uri
-                                );
-                                // For OutputOptionsType is OutputOptions.OPTIONS_TYPE_MEDIA_STORE,
-                                // the Photo/Gallery apps on devices (API Level < Q) sometimes will
-                                // not show the video files saved in MediaStore, suggest to call
-                                // scanFile still to force scan the media file.
-                                // scanVideoOutputFile(new File(videoFilePath));
-                                break;
-                            case  OutputOptions.OPTIONS_TYPE_FILE:
-                                videoFilePath = ((FileOutputOptions) finalize.getOutputOptions())
+                            );
+                            // For OutputOptionsType is OutputOptions.OPTIONS_TYPE_MEDIA_STORE,
+                            // the Photo/Gallery apps on devices (API Level < Q) sometimes will
+                            // not show the video files saved in MediaStore, suggest to call
+                            // scanFile still to force scan the media file.
+                            // scanVideoOutputFile(new File(videoFilePath));
+                        } else if (outputOptions instanceof FileOutputOptions) {
+                            videoFilePath = ((FileOutputOptions) outputOptions)
                                     .getFile().getAbsolutePath();
-                                msg = "Saved video file: " + videoFilePath;
-                                scanVideoOutputFile(new File(videoFilePath));
-                                break;
-                            case OutputOptions.OPTIONS_TYPE_FILE_DESCRIPTOR:
-                            default:
-                                throw new AssertionError("Unknown OutputOptions type: "
-                                        + finalize.getOutputOptions().getType());
+                            msg = "Saved video file: " + videoFilePath;
+                            scanVideoOutputFile(new File(videoFilePath));
+                        } else {
+                            throw new AssertionError("Unknown or unsupported OutputOptions type: "
+                                    + outputOptions.getClass().getSimpleName());
                         }
                         // The video file path is used in tracing e2e test log. Don't remove it.
                         Log.d(TAG, "Saved video file: " + videoFilePath);