[go: nahoru, domu]
Skip to content

Commit

Permalink
Always null-check MediaCodec with synchronized Holder (#3597)
Browse files Browse the repository at this point in the history 
Wraps mMediaCodec in a synchronized holder class, guaranteeing
synchronized access and prevents unhandled null pointer exceptions.

This is alternate implementation to #3596 
    
b/331835987
  • Loading branch information
@kaidokert
kaidokert committed Jun 24, 2024
1 parent 4ffd555 commit 77126bc
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 27 deletions.
53 changes: 26 additions & 27 deletions starboard/android/apk/app/src/main/java/dev/cobalt/media/MediaCodecBridge.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import android.os.Bundle;
import android.view.Surface;
import dev.cobalt.util.Log;
import dev.cobalt.util.SynchronizedHolder;
import dev.cobalt.util.UsedByNative;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
Expand Down Expand Up @@ -77,7 +78,9 @@ class MediaCodecBridge {
private static final int BITRATE_ADJUSTMENT_FPS = 30;

private long mNativeMediaCodecBridge;
private MediaCodec mMediaCodec;
private final SynchronizedHolder<MediaCodec, IllegalStateException> mMediaCodec =
new SynchronizedHolder<>(() -> new IllegalStateException("MediaCodec was destroyed"));

private MediaCodec.Callback mCallback;
private boolean mFlushed;
private long mLastPresentationTimeUs;
Expand Down Expand Up @@ -455,7 +458,7 @@ public MediaCodecBridge(
throw new IllegalArgumentException();
}
mNativeMediaCodecBridge = nativeMediaCodecBridge;
mMediaCodec = mediaCodec;
mMediaCodec.set(mediaCodec);
mMime = mime; // TODO: Delete the unused mMime field
mLastPresentationTimeUs = 0;
mFlushed = true;
Expand Down Expand Up @@ -524,7 +527,7 @@ public void onOutputFormatChanged(MediaCodec codec, MediaFormat format) {
}
}
};
mMediaCodec.setCallback(mCallback);
mMediaCodec.get().setCallback(mCallback);

if (isFrameRenderedCallbackEnabled() || tunnelModeAudioSessionId != -1) {
mFrameRendererListener =
Expand All @@ -540,7 +543,7 @@ public void onFrameRendered(MediaCodec codec, long presentationTimeUs, long nano
}
}
};
mMediaCodec.setOnFrameRenderedListener(mFrameRendererListener, null);
mMediaCodec.get().setOnFrameRenderedListener(mFrameRendererListener, null);
}
}

Expand Down Expand Up @@ -763,22 +766,22 @@ public static void createVideoMediaCodecBridge(
@UsedByNative
public void release() {
try {
String codecName = mMediaCodec.getName();
String codecName = mMediaCodec.get().getName();
Log.w(TAG, "calling MediaCodec.release() on " + codecName);
mMediaCodec.release();
mMediaCodec.get().release();
} catch (Exception e) {
// The MediaCodec is stuck in a wrong state, possibly due to losing
// the surface.
Log.e(TAG, "Cannot release media codec", e);
}
mMediaCodec = null;
mMediaCodec.set(null);
}

@SuppressWarnings("unused")
@UsedByNative
public boolean start() {
try {
mMediaCodec.start();
mMediaCodec.get().start();
} catch (IllegalStateException | IllegalArgumentException e) {
Log.e(TAG, "Cannot start the media codec", e);
return false;
Expand Down Expand Up @@ -810,15 +813,11 @@ private void updateOperatingRate() {
Log.e(TAG, "Failed to set operating rate with invalid fps " + mFps);
return;
}
if (mMediaCodec == null) {
Log.e(TAG, "Failed to set operating rate when media codec is null.");
return;
}
double operatingRate = mPlaybackRate * mFps;
Bundle b = new Bundle();
b.putFloat(MediaFormat.KEY_OPERATING_RATE, (float) operatingRate);
try {
mMediaCodec.setParameters(b);
mMediaCodec.get().setParameters(b);
} catch (IllegalStateException e) {
Log.e(TAG, "Failed to set MediaCodec operating rate", e);
}
Expand All @@ -829,7 +828,7 @@ private void updateOperatingRate() {
private int flush() {
try {
mFlushed = true;
mMediaCodec.flush();
mMediaCodec.get().flush();
} catch (Exception e) {
Log.e(TAG, "Failed to flush MediaCodec", e);
return MEDIA_CODEC_ERROR;
Expand All @@ -852,7 +851,7 @@ private void resetNativeMediaCodecBridge() {
private void stop() {
resetNativeMediaCodecBridge();
try {
mMediaCodec.stop();
mMediaCodec.get().stop();
} catch (Exception e) {
Log.e(TAG, "Failed to stop MediaCodec", e);
}
Expand All @@ -863,7 +862,7 @@ private void stop() {
private String getName() {
String codecName = "unknown";
try {
codecName = mMediaCodec.getName();
codecName = mMediaCodec.get().getName();
} catch (IllegalStateException e) {
Log.e(TAG, "Cannot get codec name", e);
}
Expand All @@ -876,7 +875,7 @@ private void getOutputFormat(GetOutputFormatResult outGetOutputFormatResult) {
MediaFormat format = null;
int status = MEDIA_CODEC_OK;
try {
format = mMediaCodec.getOutputFormat();
format = mMediaCodec.get().getOutputFormat();
} catch (IllegalStateException e) {
Log.e(TAG, "Failed to get output format", e);
status = MEDIA_CODEC_ERROR;
Expand All @@ -890,7 +889,7 @@ private void getOutputFormat(GetOutputFormatResult outGetOutputFormatResult) {
@UsedByNative
private ByteBuffer getInputBuffer(int index) {
try {
return mMediaCodec.getInputBuffer(index);
return mMediaCodec.get().getInputBuffer(index);
} catch (IllegalStateException e) {
Log.e(TAG, "Failed to get input buffer", e);
return null;
Expand All @@ -902,7 +901,7 @@ private ByteBuffer getInputBuffer(int index) {
@UsedByNative
private ByteBuffer getOutputBuffer(int index) {
try {
return mMediaCodec.getOutputBuffer(index);
return mMediaCodec.get().getOutputBuffer(index);
} catch (IllegalStateException e) {
Log.e(TAG, "Failed to get output buffer", e);
return null;
Expand All @@ -915,7 +914,7 @@ private int queueInputBuffer(
int index, int offset, int size, long presentationTimeUs, int flags) {
resetLastPresentationTimeIfNeeded(presentationTimeUs);
try {
mMediaCodec.queueInputBuffer(index, offset, size, presentationTimeUs, flags);
mMediaCodec.get().queueInputBuffer(index, offset, size, presentationTimeUs, flags);
} catch (Exception e) {
Log.e(TAG, "Failed to queue input buffer", e);
return MEDIA_CODEC_ERROR;
Expand All @@ -934,7 +933,7 @@ private void setVideoBitrate(int bps, int frameRate) {
Bundle b = new Bundle();
b.putInt(MediaCodec.PARAMETER_KEY_VIDEO_BITRATE, targetBps);
try {
mMediaCodec.setParameters(b);
mMediaCodec.get().setParameters(b);
} catch (IllegalStateException e) {
Log.e(TAG, "Failed to set MediaCodec parameters", e);
}
Expand All @@ -947,7 +946,7 @@ private void requestKeyFrameSoon() {
Bundle b = new Bundle();
b.putInt(MediaCodec.PARAMETER_KEY_REQUEST_SYNC_FRAME, 0);
try {
mMediaCodec.setParameters(b);
mMediaCodec.get().setParameters(b);
} catch (IllegalStateException e) {
Log.e(TAG, "Failed to set MediaCodec parameters", e);
}
Expand Down Expand Up @@ -980,7 +979,7 @@ private int queueSecureInputBuffer(
return MEDIA_CODEC_ERROR;
}

mMediaCodec.queueSecureInputBuffer(index, offset, cryptoInfo, presentationTimeUs, 0);
mMediaCodec.get().queueSecureInputBuffer(index, offset, cryptoInfo, presentationTimeUs, 0);
} catch (MediaCodec.CryptoException e) {
int errorCode = e.getErrorCode();
if (errorCode == MediaCodec.CryptoException.ERROR_NO_KEY) {
Expand Down Expand Up @@ -1012,7 +1011,7 @@ private int queueSecureInputBuffer(
@UsedByNative
private void releaseOutputBuffer(int index, boolean render) {
try {
mMediaCodec.releaseOutputBuffer(index, render);
mMediaCodec.get().releaseOutputBuffer(index, render);
} catch (IllegalStateException e) {
// TODO: May need to report the error to the caller. crbug.com/356498.
Log.e(TAG, "Failed to release output buffer", e);
Expand All @@ -1023,7 +1022,7 @@ private void releaseOutputBuffer(int index, boolean render) {
@UsedByNative
private void releaseOutputBuffer(int index, long renderTimestampNs) {
try {
mMediaCodec.releaseOutputBuffer(index, renderTimestampNs);
mMediaCodec.get().releaseOutputBuffer(index, renderTimestampNs);
} catch (IllegalStateException e) {
// TODO: May need to report the error to the caller. crbug.com/356498.
Log.e(TAG, "Failed to release output buffer", e);
Expand Down Expand Up @@ -1057,7 +1056,7 @@ private boolean configureVideo(
}

maybeSetMaxVideoInputSize(format);
mMediaCodec.configure(format, surface, crypto, flags);
mMediaCodec.get().configure(format, surface, crypto, flags);
mFrameRateEstimator = new FrameRateEstimator();
return true;
} catch (IllegalArgumentException e) {
Expand Down Expand Up @@ -1166,7 +1165,7 @@ private void maybeSetMaxVideoInputSize(MediaFormat format) {
@UsedByNative
public boolean configureAudio(MediaFormat format, MediaCrypto crypto, int flags) {
try {
mMediaCodec.configure(format, null, crypto, flags);
mMediaCodec.get().configure(format, null, crypto, flags);
return true;
} catch (IllegalArgumentException | IllegalStateException e) {
Log.e(TAG, "Cannot configure the audio codec", e);
Expand Down
41 changes: 41 additions & 0 deletions starboard/android/apk/app/src/main/java/dev/cobalt/util/SynchronizedHolder.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2024 The Cobalt Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package dev.cobalt.util;

import java.util.function.Supplier;

/**
* Generic holder class to turn null checks into a known RuntimeException. Access is synchronized,
* so access from multiple threads is safe.
*/
public class SynchronizedHolder<T, E extends RuntimeException> {
private T value;
private final Supplier<E> exceptionSupplier;

public SynchronizedHolder(Supplier<E> exceptionSupplier) {
this.exceptionSupplier = exceptionSupplier;
}

public synchronized T get() {
if (value == null) {
throw exceptionSupplier.get();
}
return value;
}

public synchronized void set(T value) {
this.value = value;
}
}

0 comments on commit 77126bc

Please sign in to comment.