Bug 5238515 AndroidBufferQueue miscellaneous
AndroidBufferQueue:
- errors found by setItems are now hard errors, and cause Enqueue to
return an error result (e.g. PARAMETER_INVALID or PRECONDITIONS_VIOLATED)
- disallow EOS with non-zero data
- disallow Enqueue after EOS
- Enqueue checks MPEG-2 sync byte of first packet to reduce chance of downstream failures
- use MPEG-2 terminology "packet" instead of "block"
- Clear and init don't need to erase buffer content, as it is inaccessible
- put placeholder in IAndroidBufferQueue_SetCallbackEventsMask
for additional events beyond SL_ANDROIDBUFFERQUEUEEVENT_PROCESSED
- comment unused field mBufferState in AdvancedBufferHeader
- fix a minor typo
- add dump method, #if 0 out by default
Change-Id: I11921e3784bfdb30e2cebaa1dabb705ea5ab0b92
diff --git a/src/android/android_defs.h b/src/android/android_defs.h
index 59f772a..609ef28 100644
--- a/src/android/android_defs.h
+++ b/src/android/android_defs.h
@@ -62,7 +62,8 @@
#define PLAYER_FD_FIND_FILE_SIZE ((int64_t)0xFFFFFFFFFFFFFFFFll)
-#define MPEG2_TS_BLOCK_SIZE 188
+#define MPEG2_TS_PACKET_SIZE 188
+#define MPEG2_TS_PACKET_SYNC 0x47
struct AudioPlayback_Parameters {
int streamType;
diff --git a/src/itf/IAndroidBufferQueue.c b/src/itf/IAndroidBufferQueue.c
index 76ab546..b0f79fa 100644
--- a/src/itf/IAndroidBufferQueue.c
+++ b/src/itf/IAndroidBufferQueue.c
@@ -23,7 +23,7 @@
#include "android/include/AacBqToPcmCbRenderer.h"
/**
- * Determine the state of the audio player or audio recorder associated with a buffer queue.
+ * Determine the state of the audio player or media player associated with a buffer queue.
* Note that PLAYSTATE and RECORDSTATE values are equivalent (where PLAYING == RECORDING).
*/
@@ -51,8 +51,9 @@
* parse and set the items associated with the given buffer, based on the buffer type,
* which determines the set of authorized items and format
*/
-static void setItems(const SLAndroidBufferItem *pItems, SLuint32 itemsLength,
- SLuint16 bufferType, AdvancedBufferHeader *pBuff)
+static SLresult setItems(SLuint32 dataLength,
+ const SLAndroidBufferItem *pItems, SLuint32 itemsLength,
+ SLuint16 bufferType, AdvancedBufferHeader *pBuff, bool *pEOS)
{
// reset item structure based on type
switch (bufferType) {
@@ -67,7 +68,7 @@
default:
// shouldn't happen, but just in case clear out the item structure
memset(&pBuff->mItems, 0, sizeof(AdvancedBufferItems));
- return;
+ return SL_RESULT_INTERNAL_ERROR;
}
// process all items in the array; if no items then we break out of loop immediately
@@ -75,16 +76,16 @@
// remaining length must be large enough for one full item without any associated data
if (itemsLength < sizeof(SLAndroidBufferItem)) {
- SL_LOGE("Partial item at end of array ignored");
- break;
+ SL_LOGE("Partial item at end of array");
+ return SL_RESULT_PARAMETER_INVALID;
}
itemsLength -= sizeof(SLAndroidBufferItem);
// remaining length must be large enough for data with current item and alignment padding
SLuint32 itemDataSizeWithAlignmentPadding = (pItems->itemSize + 3) & ~3;
if (itemsLength < itemDataSizeWithAlignmentPadding) {
- SL_LOGE("Partial item data at end of array ignored");
- break;
+ SL_LOGE("Partial item data at end of array");
+ return SL_RESULT_PARAMETER_INVALID;
}
itemsLength -= itemDataSizeWithAlignmentPadding;
@@ -98,8 +99,8 @@
pBuff->mItems.mTsCmdData.mTsCmdCode |= ANDROID_MP2TSEVENT_EOS;
//SL_LOGD("Found EOS event=%d", pBuff->mItems.mTsCmdData.mTsCmdCode);
if (pItems->itemSize != 0) {
- SL_LOGE("Invalid item parameter size %u for EOS, ignoring value",
- pItems->itemSize);
+ SL_LOGE("Invalid item parameter size %u for EOS", pItems->itemSize);
+ return SL_RESULT_PARAMETER_INVALID;
}
break;
@@ -112,25 +113,23 @@
pBuff->mItems.mTsCmdData.mPts = *((SLAuint64*)pItems->itemData);
//SL_LOGD("Found PTS=%lld", pBuff->mItems.mTsCmdData.mPts);
} else {
- SL_LOGE("Invalid item parameter size %u for MPEG-2 PTS, ignoring value",
- pItems->itemSize);
- pBuff->mItems.mTsCmdData.mTsCmdCode |= ANDROID_MP2TSEVENT_DISCONTINUITY;
+ SL_LOGE("Invalid item parameter size %u for MPEG-2 PTS", pItems->itemSize);
+ return SL_RESULT_PARAMETER_INVALID;
}
break;
case SL_ANDROID_ITEMKEY_FORMAT_CHANGE:
pBuff->mItems.mTsCmdData.mTsCmdCode |= ANDROID_MP2TSEVENT_FORMAT_CHANGE;
if (pItems->itemSize != 0) {
- SL_LOGE("Invalid item parameter size %u for format change, ignoring value",
- pItems->itemSize);
+ SL_LOGE("Invalid item parameter size %u for format change", pItems->itemSize);
+ return SL_RESULT_PARAMETER_INVALID;
}
break;
default:
// unknown item key
- SL_LOGE("Unknown item key %u with size %u ignored", pItems->itemKey,
- pItems->itemSize);
- break;
+ SL_LOGE("Unknown item key %u with size %u", pItems->itemKey, pItems->itemSize);
+ return SL_RESULT_PARAMETER_INVALID;
}// switch (pItems->itemKey)
} break;
@@ -141,16 +140,15 @@
case SL_ANDROID_ITEMKEY_EOS:
pBuff->mItems.mAdtsCmdData.mAdtsCmdCode |= ANDROID_ADTSEVENT_EOS;
if (pItems->itemSize != 0) {
- SL_LOGE("Invalid item parameter size %u for EOS, ignoring value",
- pItems->itemSize);
+ SL_LOGE("Invalid item parameter size %u for EOS", pItems->itemSize);
+ return SL_RESULT_PARAMETER_INVALID;
}
break;
default:
// unknown item key
- SL_LOGE("Unknown item key %u with size %u ignored", pItems->itemKey,
- pItems->itemSize);
- break;
+ SL_LOGE("Unknown item key %u with size %u", pItems->itemKey, pItems->itemSize);
+ return SL_RESULT_PARAMETER_INVALID;
}// switch (pItems->itemKey)
} break;
@@ -158,7 +156,7 @@
case kAndroidBufferTypeInvalid:
default:
// not reachable as we checked this earlier
- return;
+ return SL_RESULT_INTERNAL_ERROR;
}// switch (bufferType)
@@ -174,30 +172,43 @@
// supported Mpeg2Ts commands are mutually exclusive
switch (pBuff->mItems.mTsCmdData.mTsCmdCode) {
// single items are allowed
- case ANDROID_MP2TSEVENT_NONE:
case ANDROID_MP2TSEVENT_EOS:
+ if (dataLength > 0) {
+ SL_LOGE("Can't enqueue non-zero data with EOS");
+ return SL_RESULT_PRECONDITIONS_VIOLATED;
+ }
+ *pEOS = true;
+ break;
+ case ANDROID_MP2TSEVENT_NONE:
case ANDROID_MP2TSEVENT_DISCONTINUITY:
case ANDROID_MP2TSEVENT_DISCON_NEWPTS:
case ANDROID_MP2TSEVENT_FORMAT_CHANGE:
break;
// no combinations are allowed
default:
- SL_LOGE("Invalid combination of items; all ignored");
- pBuff->mItems.mTsCmdData.mTsCmdCode = ANDROID_MP2TSEVENT_NONE;
- break;
+ SL_LOGE("Invalid combination of items");
+ return SL_RESULT_PARAMETER_INVALID;
}
} break;
case kAndroidBufferTypeAacadts: {
// only one item supported, and thus no combination check needed
+ if (pBuff->mItems.mAdtsCmdData.mAdtsCmdCode == ANDROID_ADTSEVENT_EOS) {
+ if (dataLength > 0) {
+ SL_LOGE("Can't enqueue non-zero data with EOS");
+ return SL_RESULT_PRECONDITIONS_VIOLATED;
+ }
+ *pEOS = true;
+ }
} break;
case kAndroidBufferTypeInvalid:
default:
// not reachable as we checked this earlier
- return;
+ return SL_RESULT_INTERNAL_ERROR;
}
+ return SL_RESULT_SUCCESS;
}
@@ -241,40 +252,17 @@
// reset the queue state
thiz->mState.count = 0;
thiz->mState.index = 0;
- // reset the individual buffers
- for (XAuint16 i=0 ; i<(thiz->mNumBuffers + 1) ; i++) {
- thiz->mBufferArray[i].mDataBuffer = NULL;
- thiz->mBufferArray[i].mDataSize = 0;
- thiz->mBufferArray[i].mDataSizeConsumed = 0;
- thiz->mBufferArray[i].mBufferContext = NULL;
- thiz->mBufferArray[i].mBufferState = SL_ANDROIDBUFFERQUEUEEVENT_NONE;
- switch (thiz->mBufferType) {
- case kAndroidBufferTypeMpeg2Ts:
- thiz->mBufferArray[i].mItems.mTsCmdData.mTsCmdCode = ANDROID_MP2TSEVENT_NONE;
- thiz->mBufferArray[i].mItems.mTsCmdData.mPts = 0;
- break;
- case kAndroidBufferTypeAacadts:
- thiz->mBufferArray[i].mItems.mAdtsCmdData.mAdtsCmdCode = ANDROID_ADTSEVENT_NONE;
- break;
- default:
- result = SL_RESULT_CONTENT_UNSUPPORTED;
- }
- }
- if (SL_RESULT_SUCCESS == result) {
- // object-specific behavior for a clear
- switch (InterfaceToObjectID(thiz)) {
- case SL_OBJECTID_AUDIOPLAYER:
- result = SL_RESULT_SUCCESS;
- android_audioPlayer_androidBufferQueue_clear_l((CAudioPlayer*) thiz->mThis);
- break;
- case XA_OBJECTID_MEDIAPLAYER:
- result = SL_RESULT_SUCCESS;
- android_Player_androidBufferQueue_clear_l((CMediaPlayer*) thiz->mThis);
- break;
- default:
- result = SL_RESULT_PARAMETER_INVALID;
- }
+ // object-specific behavior for a clear
+ switch (InterfaceToObjectID(thiz)) {
+ case SL_OBJECTID_AUDIOPLAYER:
+ android_audioPlayer_androidBufferQueue_clear_l((CAudioPlayer*) thiz->mThis);
+ break;
+ case XA_OBJECTID_MEDIAPLAYER:
+ android_Player_androidBufferQueue_clear_l((CMediaPlayer*) thiz->mThis);
+ break;
+ default:
+ result = SL_RESULT_PARAMETER_INVALID;
}
interface_unlock_exclusive(thiz);
@@ -313,16 +301,24 @@
// buffer size check, can be done outside of lock because buffer type can't change
switch (thiz->mBufferType) {
case kAndroidBufferTypeMpeg2Ts:
- if (dataLength % MPEG2_TS_BLOCK_SIZE == 0) {
+ if (dataLength % MPEG2_TS_PACKET_SIZE == 0) {
+ // The downstream Stagefright MPEG-2 TS parser is sensitive to format errors,
+ // so do a quick sanity check beforehand on the first packet of the buffer.
+ // We don't check all the packets to avoid thrashing the data cache.
+ if ((dataLength > 0) && (*(SLuint8 *)pData != MPEG2_TS_PACKET_SYNC)) {
+ SL_LOGE("Error enqueueing MPEG-2 TS data: incorrect packet sync");
+ result = SL_RESULT_CONTENT_CORRUPTED;
+ SL_LEAVE_INTERFACE
+ }
break;
}
- SL_LOGE("Error enqueueing MPEG-2 TS data: size must be a multiple of %d (block size)",
- MPEG2_TS_BLOCK_SIZE);
+ SL_LOGE("Error enqueueing MPEG-2 TS data: size must be a multiple of %d (packet size)",
+ MPEG2_TS_PACKET_SIZE);
result = SL_RESULT_PARAMETER_INVALID;
SL_LEAVE_INTERFACE
break;
case kAndroidBufferTypeAacadts:
- // non-zero dataLength is permitted in case of EOS command only
+ // zero dataLength is permitted in case of EOS command only
if (dataLength > 0) {
result = android::AacBqToPcmCbRenderer::validateBufferStartEndOnFrameBoundaries(
pData, dataLength);
@@ -345,19 +341,24 @@
if ((newRear = oldRear + 1) == &thiz->mBufferArray[thiz->mNumBuffers + 1]) {
newRear = thiz->mBufferArray;
}
- if (newRear == thiz->mFront) {
+ if (thiz->mEOS) {
+ SL_LOGE("Can't enqueue after EOS");
+ result = SL_RESULT_PRECONDITIONS_VIOLATED;
+ } else if (newRear == thiz->mFront) {
result = SL_RESULT_BUFFER_INSUFFICIENT;
} else {
- oldRear->mDataBuffer = pData;
- oldRear->mDataSize = dataLength;
- oldRear->mDataSizeConsumed = 0;
- oldRear->mBufferContext = pBufferContext;
- oldRear->mBufferState = SL_ANDROIDBUFFERQUEUEEVENT_NONE;
- thiz->mRear = newRear;
- ++thiz->mState.count;
// set oldRear->mItems based on items
- setItems(pItems, itemsLength, thiz->mBufferType, oldRear);
- result = SL_RESULT_SUCCESS;
+ result = setItems(dataLength, pItems, itemsLength, thiz->mBufferType, oldRear,
+ &thiz->mEOS);
+ if (SL_RESULT_SUCCESS == result) {
+ oldRear->mDataBuffer = pData;
+ oldRear->mDataSize = dataLength;
+ oldRear->mDataSizeConsumed = 0;
+ oldRear->mBufferContext = pBufferContext;
+ //oldRear->mBufferState = TBD;
+ thiz->mRear = newRear;
+ ++thiz->mState.count;
+ }
}
// set enqueue attribute if state is PLAYING and the first buffer is enqueued
interface_unlock_exclusive_attributes(thiz, ((SL_RESULT_SUCCESS == result) &&
@@ -403,8 +404,7 @@
IAndroidBufferQueue *thiz = (IAndroidBufferQueue *) self;
interface_lock_exclusive(thiz);
// FIXME only supporting SL_ANDROIDBUFFERQUEUEEVENT_PROCESSED in this implementation
- if ((SL_ANDROIDBUFFERQUEUEEVENT_PROCESSED == eventFlags) ||
- (SL_ANDROIDBUFFERQUEUEEVENT_NONE == eventFlags)) {
+ if (!(~(SL_ANDROIDBUFFERQUEUEEVENT_PROCESSED /* | others TBD */ ) & eventFlags)) {
thiz->mCallbackEventsMask = eventFlags;
result = SL_RESULT_SUCCESS;
} else {
@@ -462,6 +462,7 @@
thiz->mBufferArray = NULL;
thiz->mFront = NULL;
thiz->mRear = NULL;
+ thiz->mEOS = false;
}
@@ -473,3 +474,98 @@
thiz->mBufferArray = NULL;
}
}
+
+
+#if 0
+// Dump the contents of an IAndroidBufferQueue to the log. This is for debugging only,
+// and is not a documented API. The associated object is locked throughout for atomicity,
+// but the log entries may be interspersed with unrelated logs.
+
+void IAndroidBufferQueue_log(IAndroidBufferQueue *thiz)
+{
+ interface_lock_shared(thiz);
+ SL_LOGI("IAndroidBufferQueue %p:", thiz);
+ SL_LOGI(" mState.count=%u mState.index=%u mCallback=%p mContext=%p",
+ thiz->mState.count, thiz->mState.index, thiz->mCallback, thiz->mContext);
+ const char *bufferTypeString;
+ switch (thiz->mBufferType) {
+ case kAndroidBufferTypeInvalid:
+ bufferTypeString = "kAndroidBufferTypeInvalid";
+ break;
+ case kAndroidBufferTypeMpeg2Ts:
+ bufferTypeString = "kAndroidBufferTypeMpeg2Ts";
+ break;
+ case kAndroidBufferTypeAacadts:
+ bufferTypeString = "kAndroidBufferTypeAacadts";
+ break;
+ default:
+ bufferTypeString = "unknown";
+ break;
+ }
+ SL_LOGI(" mCallbackEventsMask=0x%x, mBufferType=0x%x (%s), mEOS=%s",
+ thiz->mCallbackEventsMask,
+ thiz->mBufferType, bufferTypeString,
+ thiz->mEOS ? "true" : "false");
+ SL_LOGI(" mBufferArray=%p, mFront=%p (%u), mRear=%p (%u)",
+ thiz->mBufferArray,
+ thiz->mFront, thiz->mFront - thiz->mBufferArray,
+ thiz->mRear, thiz->mRear - thiz->mBufferArray);
+ SL_LOGI(" index mDataBuffer mDataSize mDataSizeConsumed mBufferContext mItems");
+ const AdvancedBufferHeader *hdr;
+ for (hdr = thiz->mFront; hdr != thiz->mRear; ) {
+ SLuint32 i = hdr - thiz->mBufferArray;
+ char itemString[32];
+ switch (thiz->mBufferType) {
+ case kAndroidBufferTypeMpeg2Ts:
+ switch (hdr->mItems.mTsCmdData.mTsCmdCode) {
+ case ANDROID_MP2TSEVENT_NONE:
+ strcpy(itemString, "NONE");
+ break;
+ case ANDROID_MP2TSEVENT_EOS:
+ strcpy(itemString, "EOS");
+ break;
+ case ANDROID_MP2TSEVENT_DISCONTINUITY:
+ strcpy(itemString, "DISCONTINUITY");
+ break;
+ case ANDROID_MP2TSEVENT_DISCON_NEWPTS:
+ snprintf(itemString, sizeof(itemString), "NEWPTS %llu",
+ hdr->mItems.mTsCmdData.mPts);
+ break;
+ case ANDROID_MP2TSEVENT_FORMAT_CHANGE:
+ strcpy(itemString, "FORMAT_CHANGE");
+ break;
+ default:
+ snprintf(itemString, sizeof(itemString), "0x%x", hdr->mItems.mTsCmdData.mTsCmdCode);
+ break;
+ }
+ break;
+ case kAndroidBufferTypeAacadts:
+ switch (hdr->mItems.mAdtsCmdData.mAdtsCmdCode) {
+ case ANDROID_ADTSEVENT_NONE:
+ strcpy(itemString, "NONE");
+ break;
+ case ANDROID_ADTSEVENT_EOS:
+ strcpy(itemString, "EOS");
+ break;
+ default:
+ snprintf(itemString, sizeof(itemString), "0x%x",
+ hdr->mItems.mAdtsCmdData.mAdtsCmdCode);
+ break;
+ }
+ break;
+ default:
+ strcpy(itemString, "");
+ break;
+ }
+ SL_LOGI(" %5u %11p %9u %17u %14p %s",
+ i, hdr->mDataBuffer, hdr->mDataSize, hdr->mDataSizeConsumed,
+ hdr->mBufferContext, itemString);
+ // mBufferState
+ if (++hdr == &thiz->mBufferArray[thiz->mNumBuffers + 1]) {
+ hdr = thiz->mBufferArray;
+ }
+ }
+ interface_unlock_shared(thiz);
+}
+
+#endif
diff --git a/src/itf/IEngine.c b/src/itf/IEngine.c
index 41de0a1..bf45a81 100644
--- a/src/itf/IEngine.c
+++ b/src/itf/IEngine.c
@@ -82,26 +82,6 @@
return SL_RESULT_CONTENT_UNSUPPORTED;
}
- // initialize ABQ memory
- for (SLuint16 i=0 ; i<(ap->mAndroidBufferQueue.mNumBuffers + 1) ; i++) {
- AdvancedBufferHeader *pBuf = &ap->mAndroidBufferQueue.mBufferArray[i];
- pBuf->mDataBuffer = NULL;
- pBuf->mDataSize = 0;
- pBuf->mDataSizeConsumed = 0;
- pBuf->mBufferContext = NULL;
- pBuf->mBufferState = SL_ANDROIDBUFFERQUEUEEVENT_NONE;
- switch (ap->mAndroidBufferQueue.mBufferType) {
- case kAndroidBufferTypeMpeg2Ts:
- pBuf->mItems.mTsCmdData.mTsCmdCode = ANDROID_MP2TSEVENT_NONE;
- pBuf->mItems.mTsCmdData.mPts = 0;
- break;
- case kAndroidBufferTypeAacadts:
- pBuf->mItems.mAdtsCmdData.mAdtsCmdCode = ANDROID_ADTSEVENT_NONE;
- break;
- default:
- return SL_RESULT_CONTENT_UNSUPPORTED;
- }
- }
ap->mAndroidBufferQueue.mFront = ap->mAndroidBufferQueue.mBufferArray;
ap->mAndroidBufferQueue.mRear = ap->mAndroidBufferQueue.mBufferArray;
}
@@ -1229,25 +1209,6 @@
result = SL_RESULT_MEMORY_FAILURE;
break;
} else {
- for (XAuint16 i=0 ; i<(nbBuffers + 1) ; i++) {
- thiz->mAndroidBufferQueue.mBufferArray[i].mDataBuffer = NULL;
- thiz->mAndroidBufferQueue.mBufferArray[i].mDataSize = 0;
- thiz->mAndroidBufferQueue.mBufferArray[i].mDataSizeConsumed = 0;
- thiz->mAndroidBufferQueue.mBufferArray[i].mBufferContext = NULL;
- thiz->mAndroidBufferQueue.mBufferArray[i].mBufferState =
- XA_ANDROIDBUFFERQUEUEEVENT_NONE;
- switch (thiz->mAndroidBufferQueue.mBufferType) {
- case kAndroidBufferTypeMpeg2Ts:
- thiz->mAndroidBufferQueue.mBufferArray[i].mItems.mTsCmdData.
- mTsCmdCode = ANDROID_MP2TSEVENT_NONE;
- thiz->mAndroidBufferQueue.mBufferArray[i].mItems.mTsCmdData.
- mPts = 0;
- break;
- default:
- result = SL_RESULT_CONTENT_UNSUPPORTED;
- break;
- }
- }
thiz->mAndroidBufferQueue.mFront =
thiz->mAndroidBufferQueue.mBufferArray;
thiz->mAndroidBufferQueue.mRear =
diff --git a/src/itfstruct.h b/src/itfstruct.h
index 8391c96..36d7565 100644
--- a/src/itfstruct.h
+++ b/src/itfstruct.h
@@ -672,6 +672,7 @@
AndroidBufferType_type mBufferType;
AdvancedBufferHeader *mBufferArray;
AdvancedBufferHeader *mFront, *mRear;
+ bool mEOS; // whether EOS has been enqueued; never reset
} IAndroidBufferQueue;
#endif
diff --git a/src/sles_allinclusive.h b/src/sles_allinclusive.h
index 513f774..0b33dcb 100644
--- a/src/sles_allinclusive.h
+++ b/src/sles_allinclusive.h
@@ -255,7 +255,8 @@
SLuint32 mDataSizeConsumed;
AdvancedBufferItems mItems;
const void *mBufferContext;
- SLuint32 mBufferState;
+ // mBufferState will be used for the other ABQ events we'll support in the future
+ // SLuint32 mBufferState;
} AdvancedBufferHeader;
#endif
