Refactors TransformationJob to handle terminal states in a more deter…

…misitic fashion (#273)
linkedin · Apr 26, 2024 · fe754dd · fe754dd
1 parent e32b6fd
commit fe754dd
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 33 deletions.
diff --git a/litr/src/main/java/com/linkedin/android/litr/TransformationJob.java b/litr/src/main/java/com/linkedin/android/litr/TransformationJob.java
@@ -114,19 +114,37 @@ void transform() throws MediaTransformationException {
             }
         } while (!completed);
 
-        release(completed);
+        if (completed) {
+            updateTargetFormatStats();
+            release();
+            marshallingTransformationListener.onCompleted(jobId, statsCollector.getStats());
+        }
     }
 
     @VisibleForTesting
     void cancel() {
-        marshallingTransformationListener.onCancelled(jobId, statsCollector.getStats());
-        release(false);
+        try {
+            updateTargetFormatStats();
+            release();
+            deleteOutputFiles();
+        } catch (Exception ex) {
+            Log.e(TAG, "cancel: ", ex);
+        } finally {
+            marshallingTransformationListener.onCancelled(jobId, statsCollector.getStats());
+        }
     }
 
     @VisibleForTesting
     protected void error(@Nullable Throwable cause) {
-        marshallingTransformationListener.onError(jobId, cause, statsCollector.getStats());
-        release(false);
+        try {
+            updateTargetFormatStats();
+            release();
+            deleteOutputFiles();
+        } catch (Exception ex) {
+            Log.e(TAG, "error: ", ex);
+        } finally {
+            marshallingTransformationListener.onError(jobId, cause, statsCollector.getStats());
+        }
     }
 
     @VisibleForTesting
@@ -232,12 +250,16 @@ boolean processNextFrame() throws TrackTranscoderException {
     }
 
     @VisibleForTesting
-    void release(boolean success) {
+    void release() {
         if (trackTranscoders != null) {
+            // Stop transcoders
             for (int track = 0; track < trackTranscoders.size(); track++) {
-                TrackTranscoder trackTranscoder = trackTranscoders.get(track);
-                trackTranscoder.stop();
-                statsCollector.setTargetFormat(track, trackTranscoder.getTargetMediaFormat());
+                try {
+                    TrackTranscoder trackTranscoder = trackTranscoders.get(track);
+                    trackTranscoder.stop();
+                } catch (Exception ex){
+                    Log.e(TAG, "release: Exception when stopping track transcoder: ", ex);
+                }
             }
         }
 
@@ -249,27 +271,42 @@ void release(boolean success) {
             mediaTargets.add(trackTransform.getMediaTarget());
         }
         for (MediaSource mediaSource : mediaSources) {
-            mediaSource.release();
-        }
-        for (MediaTarget mediaTarget : mediaTargets) {
-            mediaTarget.release();
-            if (!success) {
-                deleteOutputFile(mediaTarget.getOutputFilePath());
+            // Release media extractor
+            try {
+                mediaSource.release();
+            } catch (Exception ex) {
+                Log.e(TAG, "release: Exception when releasing media source: ", ex);
             }
         }
 
-        if (success) {
-            marshallingTransformationListener.onCompleted(jobId, statsCollector.getStats());
+        for (MediaTarget mediaTarget : mediaTargets) {
+            // Releases muxer along with its associated file descriptor.
+            mediaTarget.release();
         }
     }
 
     @VisibleForTesting
-    boolean deleteOutputFile(String outputFilePath) {
-        if (TextUtils.isEmpty(outputFilePath)) {
-            return false;
+    void deleteOutputFiles() {
+        if (trackTransforms != null) {
+            for (TrackTransform trackTransform : trackTransforms) {
+                try {
+                    String outputFilePath = trackTransform.getMediaTarget().getOutputFilePath();
+                    if (!TextUtils.isEmpty(outputFilePath)) {
+                        new File(outputFilePath).delete();
+                    }
+                } catch (Exception ex) {
+                    Log.e(TAG, "deleteOutputFiles: ", ex);
+                }
+            }
         }
+    }
 
-        File outputFile = new File(outputFilePath);
-        return outputFile.delete();
+    private void updateTargetFormatStats() {
+        if (trackTranscoders != null) {
+            for (int track = 0; track < trackTranscoders.size(); track++) {
+                TrackTranscoder trackTranscoder = trackTranscoders.get(track);
+                statsCollector.setTargetFormat(track, trackTranscoder.getTargetMediaFormat());
+            }
+        }
     }
 }
diff --git a/litr/src/test/java/com/linkedin/android/litr/TransformationJobShould.java b/litr/src/test/java/com/linkedin/android/litr/TransformationJobShould.java
@@ -208,12 +208,18 @@ public void reportErrorWhenTransformExceptionOccurs() throws Exception {
     @Test
     public void transformWhenNoErrors() throws Exception {
         transformationJob.transform();
-
-        verify(marshallingTransformationListener).onCompleted(eq(JOB_ID), ArgumentMatchers.<TrackTransformationInfo>anyList());
+        // Verify stats are updated
         verify(statsCollector).setTargetFormat(0, targetVideoFormat);
         verify(statsCollector).addSourceTrack(sourceVideoFormat);
         verify(statsCollector).addSourceTrack(sourceAudioFormat);
         verify(statsCollector).setTargetFormat(1, targetAudioFormat);
+        // Verify release of resources
+        verify(transformationJob).release();
+        // Verify output is not deleted in success scenarios
+        verify(transformationJob, never()).deleteOutputFiles();
+        // Verify invocation of completion callback with latest stats
+        verify(statsCollector).getStats();
+        verify(marshallingTransformationListener).onCompleted(eq(JOB_ID), ArgumentMatchers.<TrackTransformationInfo>anyList());
     }
 
     @Test(expected = InsufficientDiskSpaceException.class)
@@ -369,42 +375,43 @@ public void reportProgressWhenGreaterThanGranularity() throws Exception {
     public void stopTranscodersAndCleanupWhenReleasing() {
         loadTrackTranscoders();
 
-        transformationJob.release(true);
+        transformationJob.release();
 
         verify(videoTrackTranscoder).stop();
         verify(audioTrackTranscoder).stop();
         verify(mediaSource).release();
         verify(mediaTarget).release();
-        verify(marshallingTransformationListener).onCompleted(eq(JOB_ID), ArgumentMatchers.<TrackTransformationInfo>anyList());
-        verify(statsCollector).setTargetFormat(0, videoTrackTranscoder.getTargetMediaFormat());
-        verify(statsCollector).setTargetFormat(1, audioTrackTranscoder.getTargetMediaFormat());
-        verify(statsCollector).getStats();
     }
 
     @Test
-    public void reportErrorAndReleaseWhenError() {
+    public void releaseAndReportFailureWhenError() {
         loadTrackTranscoders();
 
         TrackTranscoderException exception = new TrackTranscoderException(TrackTranscoderException.Error.CODEC_IN_RELEASED_STATE);
         transformationJob.error(exception);
 
-        verify(marshallingTransformationListener).onError(eq(JOB_ID), eq(exception), ArgumentMatchers.<TrackTransformationInfo>anyList());
+        // Verify that stats are updated with latest target formats
         verify(statsCollector).setTargetFormat(0, videoTrackTranscoder.getTargetMediaFormat());
         verify(statsCollector).setTargetFormat(1, audioTrackTranscoder.getTargetMediaFormat());
+        // Verify callback invocation with latest stats
         verify(statsCollector).getStats();
+        verify(marshallingTransformationListener).onError(eq(JOB_ID), eq(exception), ArgumentMatchers.<TrackTransformationInfo>anyList());
     }
 
     @Test
-    public void reportErrorAndReleaseWhenCancelling() {
+    public void releaseAndReportStateWhenCancelling() {
         loadTrackTranscoders();
         List<TrackTransformationInfo> trackTransformationInfos = new ArrayList<>();
         when(statsCollector.getStats()).thenReturn(trackTransformationInfos);
 
         transformationJob.cancel();
 
-        verify(marshallingTransformationListener).onCancelled(eq(JOB_ID), trackTransformationInfosCaptor.capture());
+        // Verify that stats are updated with latest target formats
         verify(statsCollector).setTargetFormat(0, videoTrackTranscoder.getTargetMediaFormat());
         verify(statsCollector).setTargetFormat(1, audioTrackTranscoder.getTargetMediaFormat());
+        // Verify callback invocation with latest stats
+        verify(statsCollector).getStats();
+        verify(marshallingTransformationListener).onCancelled(eq(JOB_ID), trackTransformationInfosCaptor.capture());
         assertThat(trackTransformationInfosCaptor.getValue(), is(trackTransformationInfos));
     }