Merge "Updated comments" into jb-mr2-dev
diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h
index 6a86db6..6c1b691 100644
--- a/include/gui/BufferQueue.h
+++ b/include/gui/BufferQueue.h
@@ -48,7 +48,7 @@
     // ConsumerListener is the interface through which the BufferQueue notifies
     // the consumer of events that the consumer may wish to react to.  Because
     // the consumer will generally have a mutex that is locked during calls from
-    // teh consumer to the BufferQueue, these calls from the BufferQueue to the
+    // the consumer to the BufferQueue, these calls from the BufferQueue to the
     // consumer *MUST* be called only when the BufferQueue mutex is NOT locked.
     struct ConsumerListener : public virtual RefBase {
         // onFrameAvailable is called from queueBuffer each time an additional
@@ -104,66 +104,127 @@
             const sp<IGraphicBufferAlloc>& allocator = NULL);
     virtual ~BufferQueue();
 
+    // Query native window attributes.  The "what" values are enumerated in
+    // window.h (e.g. NATIVE_WINDOW_FORMAT).
     virtual int query(int what, int* value);
 
-    // setBufferCount updates the number of available buffer slots.  After
-    // calling this all buffer slots are both unallocated and owned by the
-    // BufferQueue object (i.e. they are not owned by the client).
+    // setBufferCount updates the number of available buffer slots.  If this
+    // method succeeds, buffer slots will be both unallocated and owned by
+    // the BufferQueue object (i.e. they are not owned by the producer or
+    // consumer).
+    //
+    // This will fail if the producer has dequeued any buffers, or if
+    // bufferCount is invalid.  bufferCount must generally be a value
+    // between the minimum undequeued buffer count and NUM_BUFFER_SLOTS
+    // (inclusive).  It may also be set to zero (the default) to indicate
+    // that the producer does not wish to set a value.  The minimum value
+    // can be obtained by calling query(NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS,
+    // ...).
+    //
+    // This may only be called by the producer.  The consumer will be told
+    // to discard buffers through the onBuffersReleased callback.
     virtual status_t setBufferCount(int bufferCount);
 
+    // requestBuffer returns the GraphicBuffer for slot N.
+    //
+    // In normal operation, this is called the first time slot N is returned
+    // by dequeueBuffer.  It must be called again if dequeueBuffer returns
+    // flags indicating that previously-returned buffers are no longer valid.
     virtual status_t requestBuffer(int slot, sp<GraphicBuffer>* buf);
 
-    // dequeueBuffer gets the next buffer slot index for the client to use. If a
-    // buffer slot is available then that slot index is written to the location
-    // pointed to by the buf argument and a status of OK is returned.  If no
-    // slot is available then a status of -EBUSY is returned and buf is
+    // dequeueBuffer gets the next buffer slot index for the producer to use.
+    // If a buffer slot is available then that slot index is written to the
+    // location pointed to by the buf argument and a status of OK is returned.
+    // If no slot is available then a status of -EBUSY is returned and buf is
     // unmodified.
     //
     // The fence parameter will be updated to hold the fence associated with
     // the buffer. The contents of the buffer must not be overwritten until the
-    // fence signals. If the fence is NULL, the buffer may be written
-    // immediately.
+    // fence signals. If the fence is Fence::NO_FENCE, the buffer may be
+    // written immediately.
     //
     // The width and height parameters must be no greater than the minimum of
     // GL_MAX_VIEWPORT_DIMS and GL_MAX_TEXTURE_SIZE (see: glGetIntegerv).
     // An error due to invalid dimensions might not be reported until
-    // updateTexImage() is called.
+    // updateTexImage() is called.  If width and height are both zero, the
+    // default values specified by setDefaultBufferSize() are used instead.
+    //
+    // The pixel formats are enumerated in graphics.h, e.g.
+    // HAL_PIXEL_FORMAT_RGBA_8888.  If the format is 0, the default format
+    // will be used.
+    //
+    // The usage argument specifies gralloc buffer usage flags.  The values
+    // are enumerated in gralloc.h, e.g. GRALLOC_USAGE_HW_RENDER.  These
+    // will be merged with the usage flags specified by setConsumerUsageBits.
+    //
+    // The return value may be a negative error value or a non-negative
+    // collection of flags.  If the flags are set, the return values are
+    // valid, but additional actions must be performed.
+    //
+    // If IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION is set, the
+    // producer must discard cached GraphicBuffer references for the slot
+    // returned in buf.
+    // If IGraphicBufferProducer::RELEASE_ALL_BUFFERS is set, the producer
+    // must discard cached GraphicBuffer references for all slots.
+    //
+    // In both cases, the producer will need to call requestBuffer to get a
+    // GraphicBuffer handle for the returned slot.
     virtual status_t dequeueBuffer(int *buf, sp<Fence>* fence,
             uint32_t width, uint32_t height, uint32_t format, uint32_t usage);
 
-    // queueBuffer returns a filled buffer to the BufferQueue. In addition, a
-    // timestamp must be provided for the buffer. The timestamp is in
+    // queueBuffer returns a filled buffer to the BufferQueue.
+    //
+    // Additional data is provided in the QueueBufferInput struct.  Notably,
+    // a timestamp must be provided for the buffer. The timestamp is in
     // nanoseconds, and must be monotonically increasing. Its other semantics
-    // (zero point, etc) are client-dependent and should be documented by the
-    // client.
+    // (zero point, etc) are producer-specific and should be documented by the
+    // producer.
+    //
+    // The caller may provide a fence that signals when all rendering
+    // operations have completed.  Alternatively, NO_FENCE may be used,
+    // indicating that the buffer is ready immediately.
+    //
+    // Some values are returned in the output struct: the current settings
+    // for default width and height, the current transform hint, and the
+    // number of queued buffers.
     virtual status_t queueBuffer(int buf,
             const QueueBufferInput& input, QueueBufferOutput* output);
 
+    // cancelBuffer returns a dequeued buffer to the BufferQueue, but doesn't
+    // queue it for use by the consumer.
+    //
+    // The buffer will not be overwritten until the fence signals.  The fence
+    // will usually be the one obtained from dequeueBuffer.
     virtual void cancelBuffer(int buf, const sp<Fence>& fence);
 
-    // setSynchronousMode set whether dequeueBuffer is synchronous or
+    // setSynchronousMode sets whether dequeueBuffer is synchronous or
     // asynchronous. In synchronous mode, dequeueBuffer blocks until
     // a buffer is available, the currently bound buffer can be dequeued and
-    // queued buffers will be retired in order.
+    // queued buffers will be acquired in order.  In asynchronous mode,
+    // a queued buffer may be replaced by a subsequently queued buffer.
+    //
     // The default mode is asynchronous.
     virtual status_t setSynchronousMode(bool enabled);
 
-    // connect attempts to connect a producer client API to the BufferQueue.
-    // This must be called before any other IGraphicBufferProducer methods are called
-    // except for getAllocator.
+    // connect attempts to connect a producer API to the BufferQueue.  This
+    // must be called before any other IGraphicBufferProducer methods are
+    // called except for getAllocator.  A consumer must already be connected.
     //
-    // This method will fail if the connect was previously called on the
-    // BufferQueue and no corresponding disconnect call was made.
+    // This method will fail if connect was previously called on the
+    // BufferQueue and no corresponding disconnect call was made (i.e. if
+    // it's still connected to a producer).
+    //
+    // APIs are enumerated in window.h (e.g. NATIVE_WINDOW_API_CPU).
     virtual status_t connect(int api, QueueBufferOutput* output);
 
-    // disconnect attempts to disconnect a producer client API from the
-    // BufferQueue. Calling this method will cause any subsequent calls to other
+    // disconnect attempts to disconnect a producer API from the BufferQueue.
+    // Calling this method will cause any subsequent calls to other
     // IGraphicBufferProducer methods to fail except for getAllocator and connect.
     // Successfully calling connect after this will allow the other methods to
     // succeed again.
     //
     // This method will fail if the the BufferQueue is not currently
-    // connected to the specified client API.
+    // connected to the specified producer API.
     virtual status_t disconnect(int api);
 
     // dump our state in a String
@@ -181,7 +242,7 @@
            mFrameNumber(0),
            mBuf(INVALID_BUFFER_SLOT) {
              mCrop.makeInvalid();
-         }
+        }
         // mGraphicBuffer points to the buffer allocated for this slot, or is NULL
         // if the buffer in this slot has been acquired in the past (see
         // BufferSlot.mAcquireCalled).
@@ -210,7 +271,7 @@
         sp<Fence> mFence;
     };
 
-    // The following public functions is the consumer facing interface
+    // The following public functions are the consumer-facing interface
 
     // acquireBuffer attempts to acquire ownership of the next pending buffer in
     // the BufferQueue.  If no buffer is pending then it returns -EINVAL.  If a
@@ -222,7 +283,9 @@
     status_t acquireBuffer(BufferItem *buffer);
 
     // releaseBuffer releases a buffer slot from the consumer back to the
-    // BufferQueue pending a fence sync.
+    // BufferQueue.  This may be done while the buffer's contents are still
+    // being accessed.  The fence will signal when the buffer is no longer
+    // in use.
     //
     // If releaseBuffer returns STALE_BUFFER_SLOT, then the consumer must free
     // any references to the just-released buffer that it might have, as if it
@@ -238,6 +301,8 @@
     // consumer may be connected, and when that consumer disconnects the
     // BufferQueue is placed into the "abandoned" state, causing most
     // interactions with the BufferQueue by the producer to fail.
+    //
+    // consumer may not be NULL.
     status_t consumerConnect(const sp<ConsumerListener>& consumer);
 
     // consumerDisconnect disconnects a consumer from the BufferQueue. All
@@ -247,22 +312,28 @@
     status_t consumerDisconnect();
 
     // getReleasedBuffers sets the value pointed to by slotMask to a bit mask
-    // indicating which buffer slots the have been released by the BufferQueue
+    // indicating which buffer slots have been released by the BufferQueue
     // but have not yet been released by the consumer.
+    //
+    // This should be called from the onBuffersReleased() callback.
     status_t getReleasedBuffers(uint32_t* slotMask);
 
     // setDefaultBufferSize is used to set the size of buffers returned by
-    // requestBuffers when a with and height of zero is requested.
+    // dequeueBuffer when a width and height of zero is requested.  Default
+    // is 1x1.
     status_t setDefaultBufferSize(uint32_t w, uint32_t h);
 
-    // setDefaultBufferCount set the buffer count. If the client has requested
-    // a buffer count using setBufferCount, the server-buffer count will
-    // take effect once the client sets the count back to zero.
+    // setDefaultMaxBufferCount sets the default value for the maximum buffer
+    // count (the initial default is 2). If the producer has requested a
+    // buffer count using setBufferCount, the default buffer count will only
+    // take effect if the producer sets the count back to zero.
+    //
+    // The count must be between 2 and NUM_BUFFER_SLOTS, inclusive.
     status_t setDefaultMaxBufferCount(int bufferCount);
 
     // setMaxAcquiredBufferCount sets the maximum number of buffers that can
-    // be acquired by the consumer at one time.  This call will fail if a
-    // producer is connected to the BufferQueue.
+    // be acquired by the consumer at one time (default 1).  This call will
+    // fail if a producer is connected to the BufferQueue.
     status_t setMaxAcquiredBufferCount(int maxAcquiredBuffers);
 
     // isSynchronousMode returns whether the BufferQueue is currently in
@@ -274,41 +345,48 @@
 
     // setDefaultBufferFormat allows the BufferQueue to create
     // GraphicBuffers of a defaultFormat if no format is specified
-    // in dequeueBuffer
+    // in dequeueBuffer.  Formats are enumerated in graphics.h; the
+    // initial default is HAL_PIXEL_FORMAT_RGBA_8888.
     status_t setDefaultBufferFormat(uint32_t defaultFormat);
 
-    // setConsumerUsageBits will turn on additional usage bits for dequeueBuffer
+    // setConsumerUsageBits will turn on additional usage bits for dequeueBuffer.
+    // These are merged with the bits passed to dequeueBuffer.  The values are
+    // enumerated in gralloc.h, e.g. GRALLOC_USAGE_HW_RENDER; the default is 0.
     status_t setConsumerUsageBits(uint32_t usage);
 
-    // setTransformHint bakes in rotation to buffers so overlays can be used
+    // setTransformHint bakes in rotation to buffers so overlays can be used.
+    // The values are enumerated in window.h, e.g.
+    // NATIVE_WINDOW_TRANSFORM_ROT_90.  The default is 0 (no transform).
     status_t setTransformHint(uint32_t hint);
 
 private:
-    // freeBufferLocked frees the resources (both GraphicBuffer and EGLImage)
-    // for the given slot.
+    // freeBufferLocked frees the GraphicBuffer and sync resources for the
+    // given slot.
     void freeBufferLocked(int index);
 
-    // freeAllBuffersLocked frees the resources (both GraphicBuffer and
-    // EGLImage) for all slots.
+    // freeAllBuffersLocked frees the GraphicBuffer and sync resources for
+    // all slots.
     void freeAllBuffersLocked();
 
-    // freeAllBuffersExceptHeadLocked frees the resources (both GraphicBuffer
-    // and EGLImage) for all slots except the head of mQueue
+    // freeAllBuffersExceptHeadLocked frees the GraphicBuffer and sync
+    // resources for all slots except the head of mQueue.
     void freeAllBuffersExceptHeadLocked();
 
-    // drainQueueLocked drains the buffer queue if we're in synchronous mode
-    // returns immediately otherwise. It returns NO_INIT if the BufferQueue
-    // became abandoned or disconnected during this call.
+    // drainQueueLocked waits for the buffer queue to empty if we're in
+    // synchronous mode, or returns immediately otherwise. It returns NO_INIT
+    // if the BufferQueue is abandoned (consumer disconnected) or disconnected
+    // (producer disconnected) during the call.
     status_t drainQueueLocked();
 
     // drainQueueAndFreeBuffersLocked drains the buffer queue if we're in
     // synchronous mode and free all buffers. In asynchronous mode, all buffers
-    // are freed except the current buffer.
+    // are freed except the currently queued buffer (if it exists).
     status_t drainQueueAndFreeBuffersLocked();
 
     // setDefaultMaxBufferCountLocked sets the maximum number of buffer slots
     // that will be used if the producer does not override the buffer slot
-    // count.
+    // count.  The count must be between 2 and NUM_BUFFER_SLOTS, inclusive.
+    // The initial default is 2.
     status_t setDefaultMaxBufferCountLocked(int count);
 
     // getMinBufferCountLocked returns the minimum number of buffers allowed
@@ -352,51 +430,56 @@
         // if no buffer has been allocated.
         sp<GraphicBuffer> mGraphicBuffer;
 
-        // mEglDisplay is the EGLDisplay used to create mEglImage.
+        // mEglDisplay is the EGLDisplay used to create EGLSyncKHR objects.
         EGLDisplay mEglDisplay;
 
         // BufferState represents the different states in which a buffer slot
-        // can be.
+        // can be.  All slots are initially FREE.
         enum BufferState {
-            // FREE indicates that the buffer is not currently being used and
-            // will not be used in the future until it gets dequeued and
-            // subsequently queued by the client.
-            // aka "owned by BufferQueue, ready to be dequeued"
+            // FREE indicates that the buffer is available to be dequeued
+            // by the producer.  The buffer may be in use by the consumer for
+            // a finite time, so the buffer must not be modified until the
+            // associated fence is signaled.
+            //
+            // The slot is "owned" by BufferQueue.  It transitions to DEQUEUED
+            // when dequeueBuffer is called.
             FREE = 0,
 
             // DEQUEUED indicates that the buffer has been dequeued by the
-            // client, but has not yet been queued or canceled. The buffer is
-            // considered 'owned' by the client, and the server should not use
-            // it for anything.
+            // producer, but has not yet been queued or canceled.  The
+            // producer may modify the buffer's contents as soon as the
+            // associated ready fence is signaled.
             //
-            // Note that when in synchronous-mode (mSynchronousMode == true),
-            // the buffer that's currently attached to the texture may be
-            // dequeued by the client.  That means that the current buffer can
-            // be in either the DEQUEUED or QUEUED state.  In asynchronous mode,
-            // however, the current buffer is always in the QUEUED state.
-            // aka "owned by producer, ready to be queued"
+            // The slot is "owned" by the producer.  It can transition to
+            // QUEUED (via queueBuffer) or back to FREE (via cancelBuffer).
             DEQUEUED = 1,
 
-            // QUEUED indicates that the buffer has been queued by the client,
-            // and has not since been made available for the client to dequeue.
-            // Attaching the buffer to the texture does NOT transition the
-            // buffer away from the QUEUED state. However, in Synchronous mode
-            // the current buffer may be dequeued by the client under some
-            // circumstances. See the note about the current buffer in the
-            // documentation for DEQUEUED.
-            // aka "owned by BufferQueue, ready to be acquired"
+            // QUEUED indicates that the buffer has been filled by the
+            // producer and queued for use by the consumer.  The buffer
+            // contents may continue to be modified for a finite time, so
+            // the contents must not be accessed until the associated fence
+            // is signaled.
+            //
+            // The slot is "owned" by BufferQueue.  It can transition to
+            // ACQUIRED (via acquireBuffer) or to FREE (if another buffer is
+            // queued in asynchronous mode).
             QUEUED = 2,
 
-            // aka "owned by consumer, ready to be released"
+            // ACQUIRED indicates that the buffer has been acquired by the
+            // consumer.  As with QUEUED, the contents must not be accessed
+            // by the consumer until the fence is signaled.
+            //
+            // The slot is "owned" by the consumer.  It transitions to FREE
+            // when releaseBuffer is called.
             ACQUIRED = 3
         };
 
         // mBufferState is the current state of this buffer slot.
         BufferState mBufferState;
 
-        // mRequestBufferCalled is used for validating that the client did
+        // mRequestBufferCalled is used for validating that the producer did
         // call requestBuffer() when told to do so. Technically this is not
-        // needed but useful for debugging and catching client bugs.
+        // needed but useful for debugging and catching producer bugs.
         bool mRequestBufferCalled;
 
         // mCrop is the current crop rectangle for this buffer slot.
@@ -414,13 +497,16 @@
         // to set by queueBuffer each time this slot is queued.
         int64_t mTimestamp;
 
-        // mFrameNumber is the number of the queued frame for this slot.
+        // mFrameNumber is the number of the queued frame for this slot.  This
+        // is used to dequeue buffers in LRU order (useful because buffers
+        // may be released before their release fence is signaled).
         uint64_t mFrameNumber;
 
         // mEglFence is the EGL sync object that must signal before the buffer
         // associated with this buffer slot may be dequeued. It is initialized
-        // to EGL_NO_SYNC_KHR when the buffer is created and (optionally, based
-        // on a compile-time option) set to a new sync object in updateTexImage.
+        // to EGL_NO_SYNC_KHR when the buffer is created and may be set to a
+        // new sync object in releaseBuffer.  (This is deprecated in favor of
+        // mFence, below.)
         EGLSyncKHR mEglFence;
 
         // mFence is a fence which will signal when work initiated by the
@@ -431,29 +517,32 @@
         // QUEUED, it indicates when the producer has finished filling the
         // buffer. When the buffer is DEQUEUED or ACQUIRED, the fence has been
         // passed to the consumer or producer along with ownership of the
-        // buffer, and mFence is empty.
+        // buffer, and mFence is set to NO_FENCE.
         sp<Fence> mFence;
 
         // Indicates whether this buffer has been seen by a consumer yet
         bool mAcquireCalled;
 
-        // Indicates whether this buffer needs to be cleaned up by consumer
+        // Indicates whether this buffer needs to be cleaned up by the
+        // consumer.  This is set when a buffer in ACQUIRED state is freed.
+        // It causes releaseBuffer to return STALE_BUFFER_SLOT.
         bool mNeedsCleanupOnRelease;
     };
 
-    // mSlots is the array of buffer slots that must be mirrored on the client
-    // side. This allows buffer ownership to be transferred between the client
-    // and server without sending a GraphicBuffer over binder. The entire array
-    // is initialized to NULL at construction time, and buffers are allocated
-    // for a slot when requestBuffer is called with that slot's index.
+    // mSlots is the array of buffer slots that must be mirrored on the
+    // producer side. This allows buffer ownership to be transferred between
+    // the producer and consumer without sending a GraphicBuffer over binder.
+    // The entire array is initialized to NULL at construction time, and
+    // buffers are allocated for a slot when requestBuffer is called with
+    // that slot's index.
     BufferSlot mSlots[NUM_BUFFER_SLOTS];
 
     // mDefaultWidth holds the default width of allocated buffers. It is used
-    // in requestBuffers() if a width and height of zero is specified.
+    // in dequeueBuffer() if a width and height of zero is specified.
     uint32_t mDefaultWidth;
 
     // mDefaultHeight holds the default height of allocated buffers. It is used
-    // in requestBuffers() if a width and height of zero is specified.
+    // in dequeueBuffer() if a width and height of zero is specified.
     uint32_t mDefaultHeight;
 
     // mMaxAcquiredBufferCount is the number of buffers that the consumer may
@@ -490,12 +579,13 @@
     // mSynchronousMode whether we're in synchronous mode or not
     bool mSynchronousMode;
 
-    // mAllowSynchronousMode whether we allow synchronous mode or not
+    // mAllowSynchronousMode whether we allow synchronous mode or not.  Set
+    // when the BufferQueue is created (by the consumer).
     const bool mAllowSynchronousMode;
 
-    // mConnectedApi indicates the API that is currently connected to this
-    // BufferQueue.  It defaults to NO_CONNECTED_API (= 0), and gets updated
-    // by the connect and disconnect methods.
+    // mConnectedApi indicates the producer API that is currently connected
+    // to this BufferQueue.  It defaults to NO_CONNECTED_API (= 0), and gets
+    // updated by the connect and disconnect methods.
     int mConnectedApi;
 
     // mDequeueCondition condition used for dequeueBuffer in synchronous mode
@@ -506,14 +596,15 @@
     Fifo mQueue;
 
     // mAbandoned indicates that the BufferQueue will no longer be used to
-    // consume images buffers pushed to it using the IGraphicBufferProducer interface.
-    // It is initialized to false, and set to true in the abandon method.  A
-    // BufferQueue that has been abandoned will return the NO_INIT error from
-    // all IGraphicBufferProducer methods capable of returning an error.
+    // consume image buffers pushed to it using the IGraphicBufferProducer
+    // interface.  It is initialized to false, and set to true in the
+    // consumerDisconnect method.  A BufferQueue that has been abandoned will
+    // return the NO_INIT error from all IGraphicBufferProducer methods
+    // capable of returning an error.
     bool mAbandoned;
 
-    // mName is a string used to identify the BufferQueue in log messages.
-    // It is set by the setName method.
+    // mConsumerName is a string used to identify the BufferQueue in log
+    // messages.  It is set by the setConsumerName method.
     String8 mConsumerName;
 
     // mMutex is the mutex used to prevent concurrent access to the member
@@ -521,12 +612,13 @@
     // member variables are accessed.
     mutable Mutex mMutex;
 
-    // mFrameCounter is the free running counter, incremented for every buffer queued
-    // with the surface Texture.
+    // mFrameCounter is the free running counter, incremented on every
+    // successful queueBuffer call.
     uint64_t mFrameCounter;
 
-    // mBufferHasBeenQueued is true once a buffer has been queued.  It is reset
-    // by changing the buffer count.
+    // mBufferHasBeenQueued is true once a buffer has been queued.  It is
+    // reset when something causes all buffers to be freed (e.g. changing the
+    // buffer count).
     bool mBufferHasBeenQueued;
 
     // mDefaultBufferFormat can be set so it will override
diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp
index 75a0296..b4c7231 100644
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -106,7 +106,7 @@
     mDefaultMaxBufferCount = count;
     mDequeueCondition.broadcast();
 
-    return OK;
+    return NO_ERROR;
 }
 
 bool BufferQueue::isSynchronousMode() const {
@@ -122,20 +122,20 @@
 status_t BufferQueue::setDefaultBufferFormat(uint32_t defaultFormat) {
     Mutex::Autolock lock(mMutex);
     mDefaultBufferFormat = defaultFormat;
-    return OK;
+    return NO_ERROR;
 }
 
 status_t BufferQueue::setConsumerUsageBits(uint32_t usage) {
     Mutex::Autolock lock(mMutex);
     mConsumerUsageBits = usage;
-    return OK;
+    return NO_ERROR;
 }
 
 status_t BufferQueue::setTransformHint(uint32_t hint) {
     ST_LOGV("setTransformHint: %02x", hint);
     Mutex::Autolock lock(mMutex);
     mTransformHint = hint;
-    return OK;
+    return NO_ERROR;
 }
 
 status_t BufferQueue::setBufferCount(int bufferCount) {
@@ -150,7 +150,8 @@
             return NO_INIT;
         }
         if (bufferCount > NUM_BUFFER_SLOTS) {
-            ST_LOGE("setBufferCount: bufferCount larger than slots available");
+            ST_LOGE("setBufferCount: bufferCount too large (max %d)",
+                    NUM_BUFFER_SLOTS);
             return BAD_VALUE;
         }
 
@@ -167,7 +168,7 @@
         if (bufferCount == 0) {
             mOverrideMaxBufferCount = 0;
             mDequeueCondition.broadcast();
-            return OK;
+            return NO_ERROR;
         }
 
         if (bufferCount < minBufferSlots) {
@@ -191,7 +192,7 @@
         listener->onBuffersReleased();
     }
 
-    return OK;
+    return NO_ERROR;
 }
 
 int BufferQueue::query(int what, int* outValue)
@@ -587,7 +588,7 @@
     if (listener != 0) {
         listener->onFrameAvailable();
     }
-    return OK;
+    return NO_ERROR;
 }
 
 void BufferQueue::cancelBuffer(int buf, const sp<Fence>& fence) {
@@ -858,7 +859,7 @@
         return NO_BUFFER_AVAILABLE;
     }
 
-    return OK;
+    return NO_ERROR;
 }
 
 status_t BufferQueue::releaseBuffer(int buf, EGLDisplay display,
@@ -889,7 +890,7 @@
     }
 
     mDequeueCondition.broadcast();
-    return OK;
+    return NO_ERROR;
 }
 
 status_t BufferQueue::consumerConnect(const sp<ConsumerListener>& consumerListener) {
@@ -900,10 +901,14 @@
         ST_LOGE("consumerConnect: BufferQueue has been abandoned!");
         return NO_INIT;
     }
+    if (consumerListener == NULL) {
+        ST_LOGE("consumerConnect: consumerListener may not be NULL");
+        return BAD_VALUE;
+    }
 
     mConsumerListener = consumerListener;
 
-    return OK;
+    return NO_ERROR;
 }
 
 status_t BufferQueue::consumerDisconnect() {
@@ -920,7 +925,7 @@
     mQueue.clear();
     freeAllBuffersLocked();
     mDequeueCondition.broadcast();
-    return OK;
+    return NO_ERROR;
 }
 
 status_t BufferQueue::getReleasedBuffers(uint32_t* slotMask) {
@@ -956,7 +961,7 @@
     Mutex::Autolock lock(mMutex);
     mDefaultWidth = w;
     mDefaultHeight = h;
-    return OK;
+    return NO_ERROR;
 }
 
 status_t BufferQueue::setDefaultMaxBufferCount(int bufferCount) {
@@ -977,7 +982,7 @@
         return INVALID_OPERATION;
     }
     mMaxAcquiredBufferCount = maxAcquiredBuffers;
-    return OK;
+    return NO_ERROR;
 }
 
 void BufferQueue::freeAllBuffersExceptHeadLocked() {