Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors TransformationJob to handle terminal states in a more determisitic fashion #273

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 59 additions & 22 deletions litr/src/main/java/com/linkedin/android/litr/TransformationJob.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}

Expand All @@ -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());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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));
}

Expand Down
Loading