Fix Audio recording bugs

pull/506/head
Mattia Iavarone 6 years ago
parent 0ee63bebfb
commit 38abe24228
  1. 29
      cameraview/src/main/java/com/otaliastudios/cameraview/internal/utils/Pool.java
  2. 51
      cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/AudioMediaEncoder.java
  3. 18
      cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoder.java

@ -9,7 +9,7 @@ import androidx.annotation.NonNull;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
/** /**
* Base class for pools of recycleable objects. * Base class for thread-safe pools of recycleable objects.
* @param <T> the object type * @param <T> the object type
*/ */
public class Pool<T> { public class Pool<T> {
@ -19,8 +19,9 @@ public class Pool<T> {
private int maxPoolSize; private int maxPoolSize;
private int activeCount; private int activeCount;
private LinkedBlockingQueue<T> mQueue; private LinkedBlockingQueue<T> queue;
private Factory<T> factory; private Factory<T> factory;
private final Object lock = new Object();
/** /**
* Used to create new instances of objects when needed. * Used to create new instances of objects when needed.
@ -37,7 +38,7 @@ public class Pool<T> {
*/ */
public Pool(int maxPoolSize, @NonNull Factory<T> factory) { public Pool(int maxPoolSize, @NonNull Factory<T> factory) {
this.maxPoolSize = maxPoolSize; this.maxPoolSize = maxPoolSize;
this.mQueue = new LinkedBlockingQueue<>(maxPoolSize); this.queue = new LinkedBlockingQueue<>(maxPoolSize);
this.factory = factory; this.factory = factory;
} }
@ -48,8 +49,10 @@ public class Pool<T> {
* @return whether the pool is empty * @return whether the pool is empty
*/ */
public boolean isEmpty() { public boolean isEmpty() {
synchronized (lock) {
return count() >= maxPoolSize; return count() >= maxPoolSize;
} }
}
/** /**
* Returns a new item, from the recycled pool if possible (if there are recycled items), * Returns a new item, from the recycled pool if possible (if there are recycled items),
@ -60,7 +63,8 @@ public class Pool<T> {
*/ */
@Nullable @Nullable
public T get() { public T get() {
T item = mQueue.poll(); synchronized (lock) {
T item = queue.poll();
if (item != null) { if (item != null) {
activeCount++; // poll decreases, this fixes activeCount++; // poll decreases, this fixes
LOG.v("GET - Reusing recycled item.", this); LOG.v("GET - Reusing recycled item.", this);
@ -76,6 +80,7 @@ public class Pool<T> {
LOG.v("GET - Creating a new item.", this); LOG.v("GET - Creating a new item.", this);
return factory.create(); return factory.create();
} }
}
/** /**
* Recycles an item after it has been used. The item should come from a previous * Recycles an item after it has been used. The item should come from a previous
@ -84,25 +89,29 @@ public class Pool<T> {
* @param item used item * @param item used item
*/ */
public void recycle(@NonNull T item) { public void recycle(@NonNull T item) {
synchronized (lock) {
LOG.v("RECYCLE - Recycling item.", this); LOG.v("RECYCLE - Recycling item.", this);
if (--activeCount < 0) { if (--activeCount < 0) {
throw new IllegalStateException("Trying to recycle an item which makes activeCount < 0." + throw new IllegalStateException("Trying to recycle an item which makes activeCount < 0." +
"This means that this or some previous items being recycled were not coming from " + "This means that this or some previous items being recycled were not coming from " +
"this pool, or some item was recycled more than once. " + this); "this pool, or some item was recycled more than once. " + this);
} }
if (!mQueue.offer(item)) { if (!queue.offer(item)) {
throw new IllegalStateException("Trying to recycle an item while the queue is full. " + throw new IllegalStateException("Trying to recycle an item while the queue is full. " +
"This means that this or some previous items being recycled were not coming from " + "This means that this or some previous items being recycled were not coming from " +
"this pool, or some item was recycled more than once. " + this); "this pool, or some item was recycled more than once. " + this);
} }
} }
}
/** /**
* Clears the pool of recycled items. * Clears the pool of recycled items.
*/ */
@CallSuper @CallSuper
public void clear() { public void clear() {
mQueue.clear(); synchronized (lock) {
queue.clear();
}
} }
/** /**
@ -114,8 +123,10 @@ public class Pool<T> {
*/ */
@SuppressWarnings("WeakerAccess") @SuppressWarnings("WeakerAccess")
public final int count() { public final int count() {
synchronized (lock) {
return activeCount() + recycledCount(); return activeCount() + recycledCount();
} }
}
/** /**
* Returns the active items managed by this pools, which means, items * Returns the active items managed by this pools, which means, items
@ -125,8 +136,10 @@ public class Pool<T> {
*/ */
@SuppressWarnings("WeakerAccess") @SuppressWarnings("WeakerAccess")
public final int activeCount() { public final int activeCount() {
synchronized (lock) {
return activeCount; return activeCount;
} }
}
/** /**
* Returns the recycled items managed by this pool, which means, items * Returns the recycled items managed by this pool, which means, items
@ -137,7 +150,9 @@ public class Pool<T> {
*/ */
@SuppressWarnings("WeakerAccess") @SuppressWarnings("WeakerAccess")
public final int recycledCount() { public final int recycledCount() {
return mQueue.size(); synchronized (lock) {
return queue.size();
}
} }
@NonNull @NonNull

@ -56,7 +56,7 @@ public class AudioMediaEncoder extends MediaEncoder {
// This value is the number of runnables that the encoder thread is allowed to be 'behind' // This value is the number of runnables that the encoder thread is allowed to be 'behind'
// the recorder thread. It's not safe to have it very large or we can end encoding A LOT AFTER // the recorder thread. It's not safe to have it very large or we can end encoding A LOT AFTER
// the actual recording. It's better to reduce this and skip recording at all. // the actual recording. It's better to reduce this and skip recording at all.
private static final int BUFFER_POOL_MAX_SIZE = 60; private static final int BUFFER_POOL_MAX_SIZE = 100;
private static long bytesToUs(int bytes) { private static long bytesToUs(int bytes) {
return (1000000L * bytes) / BYTE_RATE; return (1000000L * bytes) / BYTE_RATE;
@ -175,8 +175,19 @@ public class AudioMediaEncoder extends MediaEncoder {
private void read(boolean endOfStream) { private void read(boolean endOfStream) {
mCurrentBuffer = mByteBufferPool.get(); mCurrentBuffer = mByteBufferPool.get();
if (mCurrentBuffer == null) { if (mCurrentBuffer == null) {
LOG.e("read thread - eos:", endOfStream, "- Skipping audio frame, encoding is too slow."); // This can happen and it means that encoding is slow with respect to recording.
// Should fix the next presentation time here, but // One might be tempted to fix precisely the next frame presentation time when this happens,
// but this is not needed because the current increaseTime() algorithm will consider delays
// when they get large.
// Sleeping before returning is a good way of balancing the two operations.
// However, if endOfStream, we CAN'T lose this frame!
if (endOfStream) {
LOG.v("read thread - eos:", endOfStream, "- No buffer, retrying.");
read(true); // try again
} else {
LOG.w("read thread - eos:", endOfStream, "- Skipping audio frame, encoding is too slow.");
sleep();
}
} else { } else {
mCurrentBuffer.clear(); mCurrentBuffer.clear();
mReadBytes = mAudioRecord.read(mCurrentBuffer, FRAME_SIZE); mReadBytes = mAudioRecord.read(mCurrentBuffer, FRAME_SIZE);
@ -185,7 +196,7 @@ public class AudioMediaEncoder extends MediaEncoder {
mLastTimeUs = increaseTime(mReadBytes); mLastTimeUs = increaseTime(mReadBytes);
LOG.i("read thread - eos:", endOfStream, "- Frame PTS:", mLastTimeUs); LOG.i("read thread - eos:", endOfStream, "- Frame PTS:", mLastTimeUs);
mCurrentBuffer.limit(mReadBytes); mCurrentBuffer.limit(mReadBytes);
onBuffer(endOfStream); mEncoder.sendInputBuffer(mCurrentBuffer, mLastTimeUs, endOfStream);
} else if (mReadBytes == AudioRecord.ERROR_INVALID_OPERATION) { } else if (mReadBytes == AudioRecord.ERROR_INVALID_OPERATION) {
LOG.e("read thread - eos:", endOfStream, "- Got AudioRecord.ERROR_INVALID_OPERATION"); LOG.e("read thread - eos:", endOfStream, "- Got AudioRecord.ERROR_INVALID_OPERATION");
} else if (mReadBytes == AudioRecord.ERROR_BAD_VALUE) { } else if (mReadBytes == AudioRecord.ERROR_BAD_VALUE) {
@ -195,13 +206,13 @@ public class AudioMediaEncoder extends MediaEncoder {
} }
/** /**
* New data at position buffer.position() of size buffer.remaining() * Sleeps for a frame duration, to skip it. This can be used to slow down
* has been written into this buffer. This method should pass the data * the recording operation to balance it with encoding.
* to the consumer.
*/ */
private void onBuffer(boolean endOfStream) { private void sleep() {
LOG.v("read thread - Sending buffer to encoder thread."); try {
mEncoder.sendInputBuffer(mCurrentBuffer, mLastTimeUs, endOfStream); Thread.sleep(bytesToUs(FRAME_SIZE) / 1000);
} catch (InterruptedException ignore) {}
} }
private long increaseTime(int readBytes) { private long increaseTime(int readBytes) {
@ -309,18 +320,14 @@ public class AudioMediaEncoder extends MediaEncoder {
inputBuffer.length = readBytes; inputBuffer.length = readBytes;
inputBuffer.isEndOfStream = endOfStream; inputBuffer.isEndOfStream = endOfStream;
mPendingOps.add(inputBuffer); mPendingOps.add(inputBuffer);
performPendingOps(endOfStream);
} LOG.i("encoding thread - performing", mPendingOps.size(), "pending operations.");
while ((inputBuffer = mPendingOps.peek()) != null) {
private void performPendingOps(boolean force) { if (endOfStream) {
LOG.i("encoding thread - performing", mPendingOps.size(), "pending operations. force:", force); acquireInputBuffer(inputBuffer);
InputBuffer buffer; performPendingOp(inputBuffer);
while ((buffer = mPendingOps.peek()) != null) { } else if (tryAcquireInputBuffer(inputBuffer)) {
if (force) { performPendingOp(inputBuffer);
acquireInputBuffer(buffer);
performPendingOp(buffer);
} else if (tryAcquireInputBuffer(buffer)) {
performPendingOp(buffer);
} else { } else {
break; // Will try later. break; // Will try later.
} }

@ -138,10 +138,9 @@ abstract class MediaEncoder {
*/ */
final void prepare(@NonNull final MediaEncoderEngine.Controller controller, final long maxLengthMillis) { final void prepare(@NonNull final MediaEncoderEngine.Controller controller, final long maxLengthMillis) {
if (mState >= STATE_PREPARING) { if (mState >= STATE_PREPARING) {
LOG.e("Wrong state while preparing. Aborting.", mState); LOG.e(mName, "Wrong state while preparing. Aborting.", mState);
return; return;
} }
setState(STATE_PREPARING);
mController = controller; mController = controller;
mBufferInfo = new MediaCodec.BufferInfo(); mBufferInfo = new MediaCodec.BufferInfo();
mMaxLengthMillis = maxLengthMillis; mMaxLengthMillis = maxLengthMillis;
@ -151,6 +150,7 @@ abstract class MediaEncoder {
@Override @Override
public void run() { public void run() {
LOG.i(mName, "Prepare was called. Executing."); LOG.i(mName, "Prepare was called. Executing.");
setState(STATE_PREPARING);
onPrepare(controller, maxLengthMillis); onPrepare(controller, maxLengthMillis);
setState(STATE_PREPARED); setState(STATE_PREPARED);
} }
@ -168,15 +168,15 @@ abstract class MediaEncoder {
* NOTE: it's important to call {@link WorkerHandler#post(Runnable)} instead of run()! * NOTE: it's important to call {@link WorkerHandler#post(Runnable)} instead of run()!
*/ */
final void start() { final void start() {
if (mState < STATE_PREPARED || mState >= STATE_STARTING) {
LOG.e("Wrong state while starting. Aborting.", mState);
return;
}
setState(STATE_STARTING);
LOG.w(mName, "Start was called. Posting."); LOG.w(mName, "Start was called. Posting.");
mWorker.post(new Runnable() { mWorker.post(new Runnable() {
@Override @Override
public void run() { public void run() {
if (mState < STATE_PREPARED || mState >= STATE_STARTING) {
LOG.e(mName, "Wrong state while starting. Aborting.", mState);
return;
}
setState(STATE_STARTING);
LOG.w(mName, "Start was called. Executing."); LOG.w(mName, "Start was called. Executing.");
onStart(); onStart();
} }
@ -213,8 +213,8 @@ abstract class MediaEncoder {
* NOTE: it's important to call {@link WorkerHandler#post(Runnable)} instead of run()! * NOTE: it's important to call {@link WorkerHandler#post(Runnable)} instead of run()!
*/ */
final void stop() { final void stop() {
if (mState >= STATE_LIMIT_REACHED) { if (mState >= STATE_STOPPING) {
LOG.e("Wrong state while stopping. Aborting.", mState); LOG.e(mName, "Wrong state while stopping. Aborting.", mState);
return; return;
} }
setState(STATE_STOPPING); setState(STATE_STOPPING);

Loading…
Cancel
Save