Bug 1299068 - part 1: code refactoring/clean-up r=snorp
authorJohn Lin <jolin@mozilla.com>
Tue, 22 Nov 2016 15:36:37 +0800
changeset 325367 c871270d176ed40c4c076fe3fa43de6405270a64
parent 325366 68870b70c4665761d703461b08e921400d51b278
child 325368 956f17ac252244a485d2d1ba2b8acfd82d9d3ec8
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewerssnorp
bugs1299068
milestone53.0a1
Bug 1299068 - part 1: code refactoring/clean-up r=snorp - move all buffer related code from the Callbacks class to (Input|Output)Pocessor - don't implicitly release output buffer to codec. Do it when client calls releaseOutput() - fix buffer management problem in reset() - minor code formatting issue MozReview-Commit-ID: FmMjFBQax0s
mobile/android/base/java/org/mozilla/gecko/media/Codec.java
mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java
--- a/mobile/android/base/java/org/mozilla/gecko/media/Codec.java
+++ b/mobile/android/base/java/org/mozilla/gecko/media/Codec.java
@@ -13,157 +13,107 @@ import android.os.IBinder;
 import android.os.RemoteException;
 import android.os.TransactionTooLargeException;
 import android.util.Log;
 import android.view.Surface;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.LinkedList;
-import java.util.NoSuchElementException;
 import java.util.Queue;
 import java.util.concurrent.ConcurrentLinkedQueue;
 
 /* package */ final class Codec extends ICodec.Stub implements IBinder.DeathRecipient {
     private static final String LOGTAG = "GeckoRemoteCodec";
     private static final boolean DEBUG = false;
 
     public enum Error {
         DECODE, FATAL
-    };
+    }
 
     private final class Callbacks implements AsyncCodec.Callbacks {
-        private ICodecCallbacks mRemote;
-        private boolean mHasInputCapacitySet;
-        private boolean mHasOutputCapacitySet;
-
-        public Callbacks(ICodecCallbacks remote) {
-            mRemote = remote;
-        }
-
         @Override
         public void onInputBufferAvailable(AsyncCodec codec, int index) {
             if (mFlushing) {
                 // Flush invalidates all buffers.
                 return;
             }
-            if (!mHasInputCapacitySet) {
-                int capacity = codec.getInputBuffer(index).capacity();
-                if (capacity > 0) {
-                    mSamplePool.setInputBufferSize(capacity);
-                    mHasInputCapacitySet = true;
-                }
-            }
-            if (!mInputProcessor.onBuffer(index)) {
-                reportError(Error.FATAL, new Exception("FAIL: input buffer queue is full"));
-            }
+
+            mInputProcessor.onBuffer(index);
         }
 
         @Override
         public void onOutputBufferAvailable(AsyncCodec codec, int index, MediaCodec.BufferInfo info) {
             if (mFlushing) {
                 // Flush invalidates all buffers.
                 return;
             }
-            ByteBuffer output = codec.getOutputBuffer(index);
-            if (!mHasOutputCapacitySet) {
-                int capacity = output.capacity();
-                if (capacity > 0) {
-                    mSamplePool.setOutputBufferSize(capacity);
-                    mHasOutputCapacitySet = true;
-                }
-            }
-            Sample copy = mSamplePool.obtainOutput(info);
-            try {
-                if (info.size > 0) {
-                    copy.buffer.readFromByteBuffer(output, info.offset, info.size);
-                }
-                mSentOutputs.add(copy);
-                mRemote.onOutput(copy);
-            } catch (IOException e) {
-                Log.e(LOGTAG, "Fail to read output buffer:" + e.getMessage());
-                outputDummy(info);
-            } catch (TransactionTooLargeException ttle) {
-                Log.e(LOGTAG, "Output is too large:" + ttle.getMessage());
-                outputDummy(info);
-            } catch (RemoteException e) {
-                // Dead recipient.
-                e.printStackTrace();
-            }
 
-            mCodec.releaseOutputBuffer(index, true);
-            boolean eos = (info.flags & MediaCodec.BUFFER_FLAG_END_OF_STREAM) != 0;
-            if (DEBUG && eos) {
-                Log.d(LOGTAG, "output EOS");
-            }
-        }
-
-        private void outputDummy(MediaCodec.BufferInfo info) {
-            try {
-                if (DEBUG) Log.d(LOGTAG, "return dummy sample");
-                mRemote.onOutput(Sample.create(null, info, null));
-            } catch (RemoteException e) {
-                // Dead recipient.
-                e.printStackTrace();
-            }
+            mOutputProcessor.onBuffer(index, info);
         }
 
         @Override
         public void onError(AsyncCodec codec, int error) {
             reportError(Error.FATAL, new Exception("codec error:" + error));
         }
 
         @Override
         public void onOutputFormatChanged(AsyncCodec codec, MediaFormat format) {
-            try {
-                mRemote.onOutputFormatChanged(new FormatParam(format));
-            } catch (RemoteException re) {
-                // Dead recipient.
-                re.printStackTrace();
-            }
+            mOutputProcessor.onFormatChanged(format);
         }
     }
 
     private final class InputProcessor {
-        private Queue<Sample> mInputSamples = new LinkedList<>();
+        private boolean mHasInputCapacitySet;
         private Queue<Integer> mAvailableInputBuffers = new LinkedList<>();
         private Queue<Sample> mDequeuedSamples = new LinkedList<>();
+        private Queue<Sample> mInputSamples = new LinkedList<>();
 
         private synchronized Sample onAllocate(int size) {
             Sample sample = mSamplePool.obtainInput(size);
             mDequeuedSamples.add(sample);
             return sample;
         }
 
-        private synchronized boolean onSample(Sample sample) {
+        private synchronized void onSample(Sample sample) {
             if (sample == null) {
-                return false;
+                Log.w(LOGTAG, "WARN: null input sample");
+                return;
             }
 
             if (!sample.isEOS()) {
                 Sample temp = sample;
                 sample = mDequeuedSamples.remove();
                 sample.info = temp.info;
                 sample.cryptoInfo = temp.cryptoInfo;
                 temp.dispose();
             }
 
-            if (!mInputSamples.offer(sample)) {
-                return false;
+            if (mInputSamples.offer(sample)) {
+                feedSampleToBuffer();
+            } else {
+                reportError(Error.FATAL, new Exception("FAIL: input sample queue is full"));
             }
-            feedSampleToBuffer();
-            return true;
         }
 
-        private synchronized boolean onBuffer(int index) {
-            if (!mAvailableInputBuffers.offer(index)) {
-                return false;
+        private synchronized void onBuffer(int index) {
+            if (!mHasInputCapacitySet) {
+                int capacity = mCodec.getInputBuffer(index).capacity();
+                if (capacity > 0) {
+                    mSamplePool.setInputBufferSize(capacity);
+                    mHasInputCapacitySet = true;
+                }
             }
-            feedSampleToBuffer();
-            return true;
+
+            if (mAvailableInputBuffers.offer(index)) {
+                feedSampleToBuffer();
+            } else {
+                reportError(Error.FATAL, new Exception("FAIL: input buffer queue is full"));
+            }
+
         }
 
         private void feedSampleToBuffer() {
             while (!mAvailableInputBuffers.isEmpty() && !mInputSamples.isEmpty()) {
                 int index = mAvailableInputBuffers.poll();
                 int len = 0;
                 Sample sample = mInputSamples.poll();
                 long pts = sample.info.presentationTimeUs;
@@ -187,24 +137,118 @@ import java.util.concurrent.ConcurrentLi
                     mCodec.queueSecureInputBuffer(index, 0, cryptoInfo, pts, flags);
                 } else {
                     mCodec.queueInputBuffer(index, 0, len, pts, flags);
                 }
             }
         }
 
         private synchronized void reset() {
+            for (Sample s : mInputSamples) {
+                mSamplePool.recycleInput(s);
+            }
             mInputSamples.clear();
+
+            for (Sample s : mDequeuedSamples) {
+                mSamplePool.recycleInput(s);
+            }
+            mDequeuedSamples.clear();
+
             mAvailableInputBuffers.clear();
         }
-   }
+    }
+
+    private class OutputProcessor {
+        private boolean mHasOutputCapacitySet;
+        private Queue<Integer> mSentIndices = new LinkedList<>();
+        private Queue<Sample> mSentOutputs = new LinkedList<>();
+
+
+        private synchronized void onBuffer(int index, MediaCodec.BufferInfo info) {
+            ByteBuffer output = mCodec.getOutputBuffer(index);
+            if (!mHasOutputCapacitySet) {
+                int capacity = output.capacity();
+                if (capacity > 0) {
+                    mSamplePool.setOutputBufferSize(capacity);
+                    mHasOutputCapacitySet = true;
+                }
+            }
+            Sample copy = mSamplePool.obtainOutput(info);
+            try {
+                if (info.size > 0) {
+                    copy.buffer.readFromByteBuffer(output, info.offset, info.size);
+                }
+                mSentIndices.add(index);
+                mSentOutputs.add(copy);
+                mCallbacks.onOutput(copy);
+            } catch (IOException e) {
+                Log.e(LOGTAG, "Fail to read output buffer:" + e.getMessage());
+                outputDummy(info);
+            } catch (TransactionTooLargeException ttle) {
+                Log.e(LOGTAG, "Output is too large:" + ttle.getMessage());
+                outputDummy(info);
+            } catch (RemoteException e) {
+                // Dead recipient.
+                e.printStackTrace();
+            }
+
+            boolean eos = (info.flags & MediaCodec.BUFFER_FLAG_END_OF_STREAM) != 0;
+            if (DEBUG && eos) {
+                Log.d(LOGTAG, "output EOS");
+            }
+       }
+
+        private void outputDummy(MediaCodec.BufferInfo info) {
+            try {
+                if (DEBUG) Log.d(LOGTAG, "return dummy sample");
+                mCallbacks.onOutput(Sample.create(null, info, null));
+            } catch (RemoteException e) {
+                // Dead recipient.
+                e.printStackTrace();
+            }
+        }
+
+        private synchronized void onRelease(Sample sample) {
+            Integer i = mSentIndices.poll();
+            Sample output = mSentOutputs.poll();
+            if (i == null || output == null) {
+                Log.d(LOGTAG, "output buffer#" + i + "(" + output + ")" + ": " + sample + " already released");
+                return;
+            }
+            mCodec.releaseOutputBuffer(i, true);
+            mSamplePool.recycleOutput(output);
+
+            sample.dispose();
+        }
+
+        private void onFormatChanged(MediaFormat format) {
+            try {
+                mCallbacks.onOutputFormatChanged(new FormatParam(format));
+            } catch (RemoteException re) {
+                // Dead recipient.
+                re.printStackTrace();
+            }
+        }
+
+        private synchronized void reset() {
+            for (int i : mSentIndices) {
+                mCodec.releaseOutputBuffer(i, false);
+            }
+            mSentIndices.clear();
+            for (Sample s : mSentOutputs) {
+                mSamplePool.recycleOutput(s);
+            }
+            mSentOutputs.clear();
+        }
+    }
 
     private volatile ICodecCallbacks mCallbacks;
     private AsyncCodec mCodec;
     private InputProcessor mInputProcessor;
+    private OutputProcessor mOutputProcessor;
     private volatile boolean mFlushing = false;
     private SamplePool mSamplePool;
     private Queue<Sample> mSentOutputs = new ConcurrentLinkedQueue<>();
     // Value will be updated after configure called.
     private volatile boolean mIsAdaptivePlaybackSupported = false;
 
     public synchronized void setCallbacks(ICodecCallbacks callbacks) throws RemoteException {
         mCallbacks = callbacks;
@@ -250,33 +294,34 @@ import java.util.concurrent.ConcurrentLi
             AsyncCodec codec = AsyncCodecFactory.create(codecName);
 
             MediaCrypto crypto = RemoteMediaDrmBridgeStub.getMediaCrypto(drmStubId);
             if (DEBUG) {
                 boolean hasCrypto = crypto != null;
                 Log.d(LOGTAG, "configure mediacodec with crypto(" + hasCrypto + ") / Id :" + drmStubId);
             }
 
-            codec.setCallbacks(new Callbacks(mCallbacks), null);
+            codec.setCallbacks(new Callbacks(), null);
 
             // Video decoder should config with adaptive playback capability.
             if (surface != null) {
                 mIsAdaptivePlaybackSupported = codec.isAdaptivePlaybackSupported(
                                                    fmt.getString(MediaFormat.KEY_MIME));
                 if (mIsAdaptivePlaybackSupported) {
                     if (DEBUG) Log.d(LOGTAG, "codec supports adaptive playback  = " + mIsAdaptivePlaybackSupported);
                     // TODO: may need to find a way to not use hard code to decide the max w/h.
                     fmt.setInteger(MediaFormat.KEY_MAX_WIDTH, 1920);
                     fmt.setInteger(MediaFormat.KEY_MAX_HEIGHT, 1080);
                 }
             }
 
             codec.configure(fmt, surface, crypto, flags);
             mCodec = codec;
             mInputProcessor = new InputProcessor();
+            mOutputProcessor = new OutputProcessor();
             mSamplePool = new SamplePool(codecName);
             if (DEBUG) Log.d(LOGTAG, codec.toString() + " created");
             return true;
         } catch (Exception e) {
             if (DEBUG) Log.d(LOGTAG, "FAIL: cannot create codec -- " + codecName);
             e.printStackTrace();
             return false;
         }
@@ -284,16 +329,17 @@ import java.util.concurrent.ConcurrentLi
 
     @Override
     public synchronized boolean isAdaptivePlaybackSupported() {
         return mIsAdaptivePlaybackSupported;
     }
 
     private void releaseCodec() {
         mInputProcessor.reset();
+        mOutputProcessor.reset();
         try {
             mCodec.release();
         } catch (Exception e) {
             reportError(Error.FATAL, e);
         }
         mCodec = null;
     }
 
@@ -353,16 +399,17 @@ import java.util.concurrent.ConcurrentLi
         }
     }
 
     @Override
     public synchronized void flush() throws RemoteException {
         mFlushing = true;
         if (DEBUG) Log.d(LOGTAG, "flush " + this);
         mInputProcessor.reset();
+        mOutputProcessor.reset();
         try {
             mCodec.flush();
         } catch (Exception e) {
             reportError(Error.FATAL, e);
         }
 
         mFlushing = false;
         if (DEBUG) Log.d(LOGTAG, "flushed " + this);
@@ -370,30 +417,22 @@ import java.util.concurrent.ConcurrentLi
 
     @Override
     public synchronized Sample dequeueInput(int size) {
         return mInputProcessor.onAllocate(size);
     }
 
     @Override
     public synchronized void queueInput(Sample sample) throws RemoteException {
-        if (!mInputProcessor.onSample(sample)) {
-            reportError(Error.FATAL, new Exception("FAIL: input sample queue is full"));
-        }
+        mInputProcessor.onSample(sample);
     }
 
     @Override
     public synchronized void releaseOutput(Sample sample) {
-        try {
-            mSamplePool.recycleOutput(mSentOutputs.remove());
-        } catch (Exception e) {
-            Log.e(LOGTAG, "failed to release output:" + sample);
-            e.printStackTrace();
-        }
-        sample.dispose();
+        mOutputProcessor.onRelease(sample);
     }
 
     @Override
     public synchronized void release() throws RemoteException {
         if (DEBUG) Log.d(LOGTAG, "release " + this);
         releaseCodec();
         mSamplePool.reset();
         mSamplePool = null;
--- a/mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java
+++ b/mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java
@@ -72,17 +72,17 @@ public final class CodecProxy {
             sample.dispose();
         }
 
         @Override
         public void onError(boolean fatal) throws RemoteException {
             reportError(fatal);
         }
 
-        public void reportError(boolean fatal) {
+        private void reportError(boolean fatal) {
             mCallbacks.onError(fatal);
         }
     }
 
     @WrapForJNI
     public static CodecProxy create(MediaFormat format,
                                     Surface surface,
                                     Callbacks callbacks,