Merge "Add samplerate and bitspersample keys" am: 25b0f8dc2a
am: 69b22687b8

Change-Id: Ie62170e09a21633508ba86d2b211d7adb23f4038
diff --git a/media/libaaudio/src/fifo/FifoControllerBase.cpp b/media/libaaudio/src/fifo/FifoControllerBase.cpp
index 14a2be1..51a8e34 100644
--- a/media/libaaudio/src/fifo/FifoControllerBase.cpp
+++ b/media/libaaudio/src/fifo/FifoControllerBase.cpp
@@ -38,7 +38,7 @@
 
 fifo_frames_t FifoControllerBase::getReadIndex() {
     // % works with non-power of two sizes
-    return (fifo_frames_t) (getReadCounter() % mCapacity);
+    return (fifo_frames_t) ((uint64_t)getReadCounter() % mCapacity);
 }
 
 void FifoControllerBase::advanceReadIndex(fifo_frames_t numFrames) {
@@ -51,7 +51,7 @@
 
 fifo_frames_t FifoControllerBase::getWriteIndex() {
     // % works with non-power of two sizes
-    return (fifo_frames_t) (getWriteCounter() % mCapacity);
+    return (fifo_frames_t) ((uint64_t)getWriteCounter() % mCapacity);
 }
 
 void FifoControllerBase::advanceWriteIndex(fifo_frames_t numFrames) {
diff --git a/media/libaaudio/tests/test_atomic_fifo.cpp b/media/libaaudio/tests/test_atomic_fifo.cpp
index 0085217..0edfebe 100644
--- a/media/libaaudio/tests/test_atomic_fifo.cpp
+++ b/media/libaaudio/tests/test_atomic_fifo.cpp
@@ -23,12 +23,12 @@
 #include "fifo/FifoController.h"
 
 using android::fifo_frames_t;
+using android::fifo_counter_t;
 using android::FifoController;
 using android::FifoBuffer;
 using android::WrappingBuffer;
 
-//void foo() {
-TEST(test_fifi_controller, fifo_indices) {
+TEST(test_fifo_controller, fifo_indices) {
     // Values are arbitrary primes designed to trigger edge cases.
     constexpr int capacity = 83;
     constexpr int threshold = 47;
@@ -73,18 +73,59 @@
     ASSERT_EQ(threshold - advance2, fifoController.getEmptyFramesAvailable());
 }
 
+TEST(test_fifo_controller, fifo_wrap_around_zero) {
+    constexpr int capacity = 7; // arbitrary prime
+    constexpr int threshold = capacity;
+    FifoController fifoController(capacity, threshold);
+    ASSERT_EQ(capacity, fifoController.getCapacity());
+    ASSERT_EQ(threshold, fifoController.getThreshold());
+
+    fifoController.setReadCounter(-10); // a bit less than negative capacity
+    for (int i = 0; i < 20; i++) {
+        EXPECT_EQ(i - 10, fifoController.getReadCounter());
+        EXPECT_GE(fifoController.getReadIndex(), 0);
+        EXPECT_LT(fifoController.getReadIndex(), capacity);
+        fifoController.advanceReadIndex(1);
+    }
+
+    fifoController.setWriteCounter(-10);
+    for (int i = 0; i < 20; i++) {
+        EXPECT_EQ(i - 10, fifoController.getWriteCounter());
+        EXPECT_GE(fifoController.getWriteIndex(), 0);
+        EXPECT_LT(fifoController.getWriteIndex(), capacity);
+        fifoController.advanceWriteIndex(1);
+    }
+}
+
+
 // TODO consider using a template for other data types.
+
+// Create a big array and then use a region in the middle for the  unit tests.
+// Then we can scan the rest of the array to see if it got clobbered.
+static constexpr fifo_frames_t kBigArraySize = 1024;
+static constexpr fifo_frames_t kFifoDataOffset = 128; // starting index of FIFO data
+static constexpr int16_t       kSafeDataValue = 0x7654; // original value of BigArray
+
 class TestFifoBuffer {
 public:
     explicit TestFifoBuffer(fifo_frames_t capacity, fifo_frames_t threshold = 0)
-        : mFifoBuffer(sizeof(int16_t), capacity) {
+        : mFifoBuffer(sizeof(int16_t), capacity,
+                      &mReadIndex,
+                      &mWriteIndex,
+                      &mVeryBigArray[kFifoDataOffset]) // address of start of FIFO data
+    {
+
+        // Assume a frame is one int16_t.
         // For reading and writing.
-        mData = new int16_t[capacity];
         if (threshold <= 0) {
             threshold = capacity;
         }
         mFifoBuffer.setThreshold(threshold);
         mThreshold = threshold;
+
+        for (fifo_frames_t i = 0; i < kBigArraySize; i++) {
+            mVeryBigArray[i] = kSafeDataValue;
+        }
     }
 
     void checkMisc() {
@@ -92,26 +133,70 @@
         ASSERT_EQ(mThreshold, mFifoBuffer.getThreshold());
     }
 
+    void verifyAddressInRange(void *p, void *valid, size_t numBytes) {
+        uintptr_t p_int = (uintptr_t) p;
+        uintptr_t valid_int = (uintptr_t) valid;
+        EXPECT_GE(p_int, valid_int);
+        EXPECT_LT(p_int, (valid_int + numBytes));
+    }
+
+    void verifyStorageIntegrity() {
+        for (fifo_frames_t i = 0; i < kFifoDataOffset; i++) {
+            EXPECT_EQ(mVeryBigArray[i], kSafeDataValue);
+        }
+        fifo_frames_t firstFrameAfter = kFifoDataOffset + mFifoBuffer.getBufferCapacityInFrames();
+        for (fifo_frames_t i = firstFrameAfter; i < kBigArraySize; i++) {
+            EXPECT_EQ(mVeryBigArray[i], kSafeDataValue);
+        }
+    }
+
     // Verify that the available frames in each part add up correctly.
-    void checkWrappingBuffer() {
+    void verifyWrappingBuffer() {
         WrappingBuffer wrappingBuffer;
+
+
+        // Does the sum of the two parts match the available value returned?
+        // For EmptyRoom
         fifo_frames_t framesAvailable =
                 mFifoBuffer.getFifoControllerBase()->getEmptyFramesAvailable();
         fifo_frames_t wrapAvailable = mFifoBuffer.getEmptyRoomAvailable(&wrappingBuffer);
         EXPECT_EQ(framesAvailable, wrapAvailable);
         fifo_frames_t bothAvailable = wrappingBuffer.numFrames[0] + wrappingBuffer.numFrames[1];
         EXPECT_EQ(framesAvailable, bothAvailable);
-
+        // For FullData
         framesAvailable =
                 mFifoBuffer.getFifoControllerBase()->getFullFramesAvailable();
         wrapAvailable = mFifoBuffer.getFullDataAvailable(&wrappingBuffer);
         EXPECT_EQ(framesAvailable, wrapAvailable);
         bothAvailable = wrappingBuffer.numFrames[0] + wrappingBuffer.numFrames[1];
         EXPECT_EQ(framesAvailable, bothAvailable);
+
+        // Are frame counts in legal range?
+        fifo_frames_t capacity = mFifoBuffer.getBufferCapacityInFrames();
+        EXPECT_GE(wrappingBuffer.numFrames[0], 0);
+        EXPECT_LE(wrappingBuffer.numFrames[0], capacity);
+        EXPECT_GE(wrappingBuffer.numFrames[1], 0);
+        EXPECT_LE(wrappingBuffer.numFrames[1], capacity);
+
+        // Are addresses within the FIFO data area?
+        size_t validBytes = capacity * sizeof(int16_t);
+        if (wrappingBuffer.numFrames[0]) {
+            verifyAddressInRange(wrappingBuffer.data[0], mFifoStorage, validBytes);
+            uint8_t *last = ((uint8_t *)wrappingBuffer.data[0])
+                            + mFifoBuffer.convertFramesToBytes(wrappingBuffer.numFrames[0]) - 1;
+            verifyAddressInRange(last, mFifoStorage, validBytes);
+        }
+        if (wrappingBuffer.numFrames[1]) {
+            verifyAddressInRange(wrappingBuffer.data[1], mFifoStorage, validBytes);
+            uint8_t *last = ((uint8_t *)wrappingBuffer.data[1])
+                            + mFifoBuffer.convertFramesToBytes(wrappingBuffer.numFrames[1]) - 1;
+            verifyAddressInRange(last, mFifoStorage, validBytes);
+        }
+
     }
 
     // Write data but do not overflow.
-    void writeData(fifo_frames_t numFrames) {
+    void writeMultipleDataFrames(fifo_frames_t numFrames) {
         fifo_frames_t framesAvailable =
                 mFifoBuffer.getFifoControllerBase()->getEmptyFramesAvailable();
         fifo_frames_t framesToWrite = std::min(framesAvailable, numFrames);
@@ -122,8 +207,8 @@
         ASSERT_EQ(framesToWrite, actual);
     }
 
-    // Read data but do not underflow.
-    void verifyData(fifo_frames_t numFrames) {
+    // Read whatever data is available, Do not underflow.
+    void verifyMultipleDataFrames(fifo_frames_t numFrames) {
         fifo_frames_t framesAvailable =
                 mFifoBuffer.getFifoControllerBase()->getFullFramesAvailable();
         fifo_frames_t framesToRead = std::min(framesAvailable, numFrames);
@@ -134,20 +219,35 @@
         }
     }
 
+    // Read specified number of frames
+    void verifyRequestedData(fifo_frames_t numFrames) {
+        fifo_frames_t framesAvailable =
+                mFifoBuffer.getFifoControllerBase()->getFullFramesAvailable();
+        ASSERT_LE(numFrames, framesAvailable);
+        fifo_frames_t framesToRead = std::min(framesAvailable, numFrames);
+        fifo_frames_t actual = mFifoBuffer.read(mData, framesToRead);
+        ASSERT_EQ(actual, numFrames);
+        for (int i = 0; i < actual; i++) {
+            ASSERT_EQ(mNextVerifyIndex++, mData[i]);
+        }
+    }
+
     // Wrap around the end of the buffer.
     void checkWrappingWriteRead() {
         constexpr int frames1 = 43;
         constexpr int frames2 = 15;
 
-        writeData(frames1);
-        checkWrappingBuffer();
-        verifyData(frames1);
-        checkWrappingBuffer();
+        writeMultipleDataFrames(frames1);
+        verifyWrappingBuffer();
+        verifyRequestedData(frames1);
+        verifyWrappingBuffer();
 
-        writeData(frames2);
-        checkWrappingBuffer();
-        verifyData(frames2);
-        checkWrappingBuffer();
+        writeMultipleDataFrames(frames2);
+        verifyWrappingBuffer();
+        verifyRequestedData(frames2);
+        verifyWrappingBuffer();
+
+        verifyStorageIntegrity();
     }
 
     // Write and Read a specific amount of data.
@@ -156,10 +256,12 @@
         // Wrap around with the smaller region in the second half.
         const int frames1 = capacity - 4;
         const int frames2 = 7; // arbitrary, small
-        writeData(frames1);
-        verifyData(frames1);
-        writeData(frames2);
-        verifyData(frames2);
+        writeMultipleDataFrames(frames1);
+        verifyRequestedData(frames1);
+        writeMultipleDataFrames(frames2);
+        verifyRequestedData(frames2);
+
+        verifyStorageIntegrity();
     }
 
     // Write and Read a specific amount of data.
@@ -168,10 +270,12 @@
         // Wrap around with the larger region in the second half.
         const int frames1 = capacity - 4;
         const int frames2 = capacity - 9; // arbitrary, large
-        writeData(frames1);
-        verifyData(frames1);
-        writeData(frames2);
-        verifyData(frames2);
+        writeMultipleDataFrames(frames1);
+        verifyRequestedData(frames1);
+        writeMultipleDataFrames(frames2);
+        verifyRequestedData(frames2);
+
+        verifyStorageIntegrity();
     }
 
     // Randomly read or write up to the maximum amount of data.
@@ -180,30 +284,67 @@
             fifo_frames_t framesEmpty =
                     mFifoBuffer.getFifoControllerBase()->getEmptyFramesAvailable();
             fifo_frames_t numFrames = (fifo_frames_t)(drand48() * framesEmpty);
-            writeData(numFrames);
+            writeMultipleDataFrames(numFrames);
 
             fifo_frames_t framesFull =
                     mFifoBuffer.getFifoControllerBase()->getFullFramesAvailable();
             numFrames = (fifo_frames_t)(drand48() * framesFull);
-            verifyData(numFrames);
+            verifyMultipleDataFrames(numFrames);
         }
+
+        verifyStorageIntegrity();
+    }
+
+    // Write and Read a specific amount of data.
+    void checkNegativeCounters() {
+        fifo_counter_t counter = -9876;
+        mFifoBuffer.setWriteCounter(counter);
+        mFifoBuffer.setReadCounter(counter);
+        checkWrappingWriteRead();
+    }
+
+    // Wrap over the boundary at 0x7FFFFFFFFFFFFFFF
+    // Note that the behavior of a signed overflow is technically undefined.
+    void checkHalfWrap() {
+        fifo_counter_t counter = INT64_MAX - 10;
+        mFifoBuffer.setWriteCounter(counter);
+        mFifoBuffer.setReadCounter(counter);
+        ASSERT_GT(mFifoBuffer.getWriteCounter(), 0);
+        checkWrappingWriteRead();
+        ASSERT_LT(mFifoBuffer.getWriteCounter(), 0); // did we wrap past INT64_MAX?
+    }
+
+    // Wrap over the boundary at 0xFFFFFFFFFFFFFFFF
+    void checkFullWrap() {
+        fifo_counter_t counter = -10;
+        mFifoBuffer.setWriteCounter(counter);
+        mFifoBuffer.setReadCounter(counter);
+        ASSERT_LT(mFifoBuffer.getWriteCounter(), 0);
+        writeMultipleDataFrames(20);
+        ASSERT_GT(mFifoBuffer.getWriteCounter(), 0); // did we wrap past zero?
+        verifyStorageIntegrity();
     }
 
     FifoBuffer     mFifoBuffer;
-    int16_t       *mData;
     fifo_frames_t  mNextWriteIndex = 0;
     fifo_frames_t  mNextVerifyIndex = 0;
     fifo_frames_t  mThreshold;
+
+    fifo_counter_t mReadIndex = 0;
+    fifo_counter_t mWriteIndex = 0;
+    int16_t        mVeryBigArray[kBigArraySize]; // Use the middle of this array for the FIFO.
+    int16_t       *mFifoStorage = &mVeryBigArray[kFifoDataOffset]; // Start here for storage.
+    int16_t        mData[kBigArraySize]{};
 };
 
-TEST(test_fifo_buffer, fifo_read_write) {
+TEST(test_fifo_buffer, fifo_write_read) {
     constexpr int capacity = 51; // arbitrary
     TestFifoBuffer tester(capacity);
     tester.checkMisc();
     tester.checkWriteRead();
 }
 
-TEST(test_fifo_buffer, fifo_wrapping_read_write) {
+TEST(test_fifo_buffer, fifo_wrapping_write_read) {
     constexpr int capacity = 59; // arbitrary, a little bigger this time
     TestFifoBuffer tester(capacity);
     tester.checkWrappingWriteRead();
@@ -227,3 +368,21 @@
     TestFifoBuffer tester(capacity, threshold);
     tester.checkRandomWriteRead();
 }
+
+TEST(test_fifo_buffer, fifo_negative_counters) {
+    constexpr int capacity = 49; // arbitrary
+    TestFifoBuffer tester(capacity);
+    tester.checkNegativeCounters();
+}
+
+TEST(test_fifo_buffer, fifo_half_wrap) {
+    constexpr int capacity = 57; // arbitrary
+    TestFifoBuffer tester(capacity);
+    tester.checkHalfWrap();
+}
+
+TEST(test_fifo_buffer, fifo_full_wrap) {
+    constexpr int capacity = 57; // arbitrary
+    TestFifoBuffer tester(capacity);
+    tester.checkFullWrap();
+}
diff --git a/media/libaudiohal/HalDeathHandlerHidl.cpp b/media/libaudiohal/HalDeathHandlerHidl.cpp
index 1e3ab58..6e33523 100644
--- a/media/libaudiohal/HalDeathHandlerHidl.cpp
+++ b/media/libaudiohal/HalDeathHandlerHidl.cpp
@@ -54,7 +54,7 @@
         handler.second();
     }
     ALOGE("HAL server crashed, audio server is restarting");
-    exit(1);
+    _exit(1);  // Avoid calling atexit handlers, as this code runs on a thread from RPC threadpool.
 }
 
 } // namespace android
diff --git a/media/libmediaplayer2/nuplayer2/NuPlayer2Driver.cpp b/media/libmediaplayer2/nuplayer2/NuPlayer2Driver.cpp
index 8fbd8dd..398c246 100644
--- a/media/libmediaplayer2/nuplayer2/NuPlayer2Driver.cpp
+++ b/media/libmediaplayer2/nuplayer2/NuPlayer2Driver.cpp
@@ -327,7 +327,7 @@
     switch (mState) {
         case STATE_RUNNING:
             mPlayer->pause();
-            [[fallthrough]];
+            // fall through
 
         case STATE_PAUSED:
             mState = STATE_STOPPED;
diff --git a/media/libstagefright/xmlparser/MediaCodecsXmlParser.cpp b/media/libstagefright/xmlparser/MediaCodecsXmlParser.cpp
index 0832944..2dec9fa 100644
--- a/media/libstagefright/xmlparser/MediaCodecsXmlParser.cpp
+++ b/media/libstagefright/xmlparser/MediaCodecsXmlParser.cpp
@@ -92,18 +92,18 @@
 status_t limitFoundMissingAttr(const char* name, const char *attr, bool found = true) {
     ALOGE("limit '%s' with %s'%s' attribute", name,
             (found ? "" : "no "), attr);
-    return -EINVAL;
+    return BAD_VALUE;
 }
 
 status_t limitError(const char* name, const char *msg) {
     ALOGE("limit '%s' %s", name, msg);
-    return -EINVAL;
+    return BAD_VALUE;
 }
 
 status_t limitInvalidAttr(const char* name, const char* attr, const char* value) {
     ALOGE("limit '%s' with invalid '%s' attribute (%s)", name,
             attr, value);
-    return -EINVAL;
+    return BAD_VALUE;
 }
 
 }; // unnamed namespace
@@ -232,12 +232,12 @@
     while (attrs[i] != nullptr) {
         if (strEq(attrs[i], "href")) {
             if (attrs[++i] == nullptr) {
-                return -EINVAL;
+                return BAD_VALUE;
             }
             href = attrs[i];
         } else {
             ALOGE("includeXMLFile: unrecognized attribute: %s", attrs[i]);
-            return -EINVAL;
+            return BAD_VALUE;
         }
         ++i;
     }
@@ -252,32 +252,32 @@
             continue;
         }
         ALOGE("invalid include file name: %s", href);
-        return -EINVAL;
+        return BAD_VALUE;
     }
 
     std::string filename = href;
     if (filename.compare(0, 13, "media_codecs_") != 0 ||
             filename.compare(filename.size() - 4, 4, ".xml") != 0) {
         ALOGE("invalid include file name: %s", href);
-        return -EINVAL;
+        return BAD_VALUE;
     }
     filename.insert(0, mHrefBase);
 
+    status_t oldParsingStatus = mParsingStatus;
+
     parseXMLFile(filename.c_str());
-    return mParsingStatus;
+
+    status_t newParsingStatus = mParsingStatus;
+    mParsingStatus = oldParsingStatus;
+    return newParsingStatus;
 }
 
 void MediaCodecsXmlParser::startElementHandler(
         const char *name, const char **attrs) {
-    if (mParsingStatus != OK) {
-        return;
-    }
-
     bool inType = true;
 
     if (strEq(name, "Include")) {
-        mParsingStatus = includeXMLFile(attrs);
-        if (mParsingStatus == OK) {
+        if (includeXMLFile(attrs) == OK) {
             mSectionStack.push_back(mCurrentSection);
             mCurrentSection = SECTION_INCLUDE;
         }
@@ -300,7 +300,7 @@
         case SECTION_SETTINGS:
         {
             if (strEq(name, "Setting")) {
-                mParsingStatus = addSettingFromAttributes(attrs);
+                (void)addSettingFromAttributes(attrs);
             }
             break;
         }
@@ -308,9 +308,7 @@
         case SECTION_DECODERS:
         {
             if (strEq(name, "MediaCodec")) {
-                mParsingStatus =
-                    addMediaCodecFromAttributes(false /* encoder */, attrs);
-
+                (void)addMediaCodecFromAttributes(false /* encoder */, attrs);
                 mCurrentSection = SECTION_DECODER;
             }
             break;
@@ -319,9 +317,7 @@
         case SECTION_ENCODERS:
         {
             if (strEq(name, "MediaCodec")) {
-                mParsingStatus =
-                    addMediaCodecFromAttributes(true /* encoder */, attrs);
-
+                (void)addMediaCodecFromAttributes(true /* encoder */, attrs);
                 mCurrentSection = SECTION_ENCODER;
             }
             break;
@@ -331,9 +327,9 @@
         case SECTION_ENCODER:
         {
             if (strEq(name, "Quirk")) {
-                mParsingStatus = addQuirk(attrs);
+                (void)addQuirk(attrs);
             } else if (strEq(name, "Type")) {
-                mParsingStatus = addTypeFromAttributes(attrs,
+                (void)addTypeFromAttributes(attrs,
                         (mCurrentSection == SECTION_ENCODER));
                 mCurrentSection =
                         (mCurrentSection == SECTION_DECODER ?
@@ -353,9 +349,9 @@
                     (strEq(name, "Limit") || strEq(name, "Feature"))) {
                 ALOGW("ignoring %s specified outside of a Type", name);
             } else if (strEq(name, "Limit")) {
-                mParsingStatus = addLimit(attrs);
+                (void)addLimit(attrs);
             } else if (strEq(name, "Feature")) {
-                mParsingStatus = addFeature(attrs);
+                (void)addFeature(attrs);
             }
             break;
         }
@@ -367,10 +363,6 @@
 }
 
 void MediaCodecsXmlParser::endElementHandler(const char *name) {
-    if (mParsingStatus != OK) {
-        return;
-    }
-
     switch (mCurrentSection) {
         case SECTION_SETTINGS:
         {
@@ -452,31 +444,31 @@
         if (strEq(attrs[i], "name")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addSettingFromAttributes: name is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             name = attrs[i];
         } else if (strEq(attrs[i], "value")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addSettingFromAttributes: value is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             value = attrs[i];
         } else if (strEq(attrs[i], "update")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addSettingFromAttributes: update is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             update = attrs[i];
         } else {
             ALOGE("addSettingFromAttributes: unrecognized attribute: %s", attrs[i]);
-            return -EINVAL;
+            return BAD_VALUE;
         }
         ++i;
     }
 
     if (name == nullptr || value == nullptr) {
         ALOGE("addSettingFromAttributes: name or value unspecified");
-        return -EINVAL;
+        return BAD_VALUE;
     }
 
     // Boolean values are converted to "0" or "1".
@@ -489,7 +481,7 @@
     if (attribute == mServiceAttributeMap.end()) { // New attribute name
         if (mUpdate) {
             ALOGE("addSettingFromAttributes: updating non-existing setting");
-            return -EINVAL;
+            return BAD_VALUE;
         }
         mServiceAttributeMap.insert(Attribute(name, value));
     } else { // Existing attribute name
@@ -513,39 +505,40 @@
         if (strEq(attrs[i], "name")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addMediaCodecFromAttributes: name is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             name = attrs[i];
         } else if (strEq(attrs[i], "type")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addMediaCodecFromAttributes: type is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             type = attrs[i];
         } else if (strEq(attrs[i], "update")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addMediaCodecFromAttributes: update is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             update = attrs[i];
         } else {
             ALOGE("addMediaCodecFromAttributes: unrecognized attribute: %s", attrs[i]);
-            return -EINVAL;
+            return BAD_VALUE;
         }
         ++i;
     }
 
     if (name == nullptr) {
         ALOGE("addMediaCodecFromAttributes: name not found");
-        return -EINVAL;
+        return BAD_VALUE;
     }
 
     mUpdate = (update != nullptr) && parseBoolean(update);
     mCurrentCodec = mCodecMap.find(name);
     if (mCurrentCodec == mCodecMap.end()) { // New codec name
         if (mUpdate) {
-            ALOGE("addMediaCodecFromAttributes: updating non-existing codec");
-            return -EINVAL;
+            ALOGW("addMediaCodecFromAttributes: cannot update "
+                  "non-existing codec \"%s\".", name);
+            return BAD_VALUE;
         }
         // Create a new codec in mCodecMap
         mCurrentCodec = mCodecMap.insert(
@@ -560,18 +553,26 @@
         mCurrentCodec->second.order = mCodecCounter++;
     } else { // Existing codec name
         if (!mUpdate) {
-            ALOGE("addMediaCodecFromAttributes: adding existing codec");
-            return -EINVAL;
+            ALOGW("addMediaCodecFromAttributes: trying to add "
+                  "existing codec \"%s\"", name);
+            return ALREADY_EXISTS;
         }
         if (type != nullptr) {
             mCurrentType = mCurrentCodec->second.typeMap.find(type);
             if (mCurrentType == mCurrentCodec->second.typeMap.end()) {
-                ALOGE("addMediaCodecFromAttributes: updating non-existing type");
-                return -EINVAL;
+                ALOGE("addMediaCodecFromAttributes: cannot update "
+                      "non-existing type \"%s\" for codec \"%s\"",
+                        type, name);
+                return BAD_VALUE;
             }
         } else {
             // This should happen only when the codec has at most one type.
             mCurrentType = mCurrentCodec->second.typeMap.begin();
+            if (mCurrentType == mCurrentCodec->second.typeMap.end()) {
+                ALOGE("addMediaCodecFromAttributes: cannot update "
+                      "codec \"%s\" without type specified", name);
+                return BAD_VALUE;
+            }
         }
     }
 
@@ -579,6 +580,10 @@
 }
 
 status_t MediaCodecsXmlParser::addQuirk(const char **attrs) {
+    if (mCurrentCodec == mCodecMap.end()) {
+        return BAD_VALUE;
+    }
+
     const char *name = nullptr;
 
     size_t i = 0;
@@ -586,19 +591,19 @@
         if (strEq(attrs[i], "name")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addQuirk: name is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             name = attrs[i];
         } else {
             ALOGE("addQuirk: unrecognized attribute: %s", attrs[i]);
-            return -EINVAL;
+            return BAD_VALUE;
         }
         ++i;
     }
 
     if (name == nullptr) {
         ALOGE("addQuirk: name not found");
-        return -EINVAL;
+        return BAD_VALUE;
     }
 
     mCurrentCodec->second.quirkSet.emplace(name);
@@ -606,6 +611,10 @@
 }
 
 status_t MediaCodecsXmlParser::addTypeFromAttributes(const char **attrs, bool encoder) {
+    if (mCurrentCodec == mCodecMap.end()) {
+        return BAD_VALUE;
+    }
+
     const char *name = nullptr;
     const char *update = nullptr;
 
@@ -614,42 +623,51 @@
         if (strEq(attrs[i], "name")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addTypeFromAttributes: name is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             name = attrs[i];
         } else if (strEq(attrs[i], "update")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addTypeFromAttributes: update is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             update = attrs[i];
         } else {
             ALOGE("addTypeFromAttributes: unrecognized attribute: %s", attrs[i]);
-            return -EINVAL;
+            return BAD_VALUE;
         }
         ++i;
     }
 
     if (name == nullptr) {
-        return -EINVAL;
+        return BAD_VALUE;
     }
 
     mCurrentCodec->second.isEncoder = encoder;
     mCurrentType = mCurrentCodec->second.typeMap.find(name);
     if (!mUpdate) {
         if (mCurrentType != mCurrentCodec->second.typeMap.end()) {
-            ALOGE("addTypeFromAttributes: re-defining existing type without update");
-            return -EINVAL;
+            ALOGW("addTypeFromAttributes: trying to update "
+                  "existing type \"%s\"", name);
+            return ALREADY_EXISTS;
         }
         mCurrentType = mCurrentCodec->second.typeMap.insert(
                 Type(name, AttributeMap())).first;
     } else if (mCurrentType == mCurrentCodec->second.typeMap.end()) {
         ALOGE("addTypeFromAttributes: updating non-existing type");
+        return BAD_VALUE;
     }
     return OK;
 }
 
 status_t MediaCodecsXmlParser::addLimit(const char **attrs) {
+    if (mCurrentCodec == mCodecMap.end()) {
+        return BAD_VALUE;
+    }
+    if (mCurrentType == mCurrentCodec->second.typeMap.end()) {
+        return BAD_VALUE;
+    }
+
     const char* a_name = nullptr;
     const char* a_default = nullptr;
     const char* a_in = nullptr;
@@ -665,78 +683,73 @@
         if (strEq(attrs[i], "name")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: name is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_name = attrs[i];
         } else if (strEq(attrs[i], "default")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: default is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_default = attrs[i];
         } else if (strEq(attrs[i], "in")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: in is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_in = attrs[i];
         } else if (strEq(attrs[i], "max")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: max is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_max = attrs[i];
         } else if (strEq(attrs[i], "min")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: min is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_min = attrs[i];
         } else if (strEq(attrs[i], "range")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: range is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_range = attrs[i];
         } else if (strEq(attrs[i], "ranges")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: ranges is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_ranges = attrs[i];
         } else if (strEq(attrs[i], "scale")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: scale is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_scale = attrs[i];
         } else if (strEq(attrs[i], "value")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addLimit: value is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             a_value = attrs[i];
         } else {
             ALOGE("addLimit: unrecognized limit: %s", attrs[i]);
-            return -EINVAL;
+            return BAD_VALUE;
         }
         ++i;
     }
 
     if (a_name == nullptr) {
         ALOGE("limit with no 'name' attribute");
-        return -EINVAL;
+        return BAD_VALUE;
     }
 
     // size, blocks, bitrate, frame-rate, blocks-per-second, aspect-ratio,
     // measured-frame-rate, measured-blocks-per-second: range
     // quality: range + default + [scale]
     // complexity: range + default
-    if (mCurrentType == mCurrentCodec->second.typeMap.end()) {
-        ALOGW("ignoring null type");
-        return OK;
-    }
-
     std::string range;
     if (strEq(a_name, "aspect-ratio") ||
             strEq(a_name, "bitrate") ||
@@ -880,6 +893,13 @@
 }
 
 status_t MediaCodecsXmlParser::addFeature(const char **attrs) {
+    if (mCurrentCodec == mCodecMap.end()) {
+        return BAD_VALUE;
+    }
+    if (mCurrentType == mCurrentCodec->second.typeMap.end()) {
+        return BAD_VALUE;
+    }
+
     size_t i = 0;
     const char *name = nullptr;
     int32_t optional = -1;
@@ -890,30 +910,30 @@
         if (strEq(attrs[i], "name")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addFeature: name is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             name = attrs[i];
         } else if (strEq(attrs[i], "optional")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addFeature: optional is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             optional = parseBoolean(attrs[i]) ? 1 : 0;
         } else if (strEq(attrs[i], "required")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addFeature: required is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             required = parseBoolean(attrs[i]) ? 1 : 0;
         } else if (strEq(attrs[i], "value")) {
             if (attrs[++i] == nullptr) {
                 ALOGE("addFeature: value is null");
-                return -EINVAL;
+                return BAD_VALUE;
             }
             value = attrs[i];
         } else {
             ALOGE("addFeature: unrecognized attribute: %s", attrs[i]);
-            return -EINVAL;
+            return BAD_VALUE;
         }
         ++i;
     }
@@ -921,23 +941,18 @@
     // Every feature must have a name.
     if (name == nullptr) {
         ALOGE("feature with no 'name' attribute");
-        return -EINVAL;
-    }
-
-    if (mCurrentType == mCurrentCodec->second.typeMap.end()) {
-        ALOGW("ignoring null type");
-        return OK;
+        return BAD_VALUE;
     }
 
     if ((optional != -1) || (required != -1)) {
         if (optional == required) {
             ALOGE("feature '%s' is both/neither optional and required", name);
-            return -EINVAL;
+            return BAD_VALUE;
         }
         if ((optional == 1) || (required == 1)) {
             if (value != nullptr) {
                 ALOGE("feature '%s' cannot have extra 'value'", name);
-                return -EINVAL;
+                return BAD_VALUE;
             }
             mCurrentType->second[std::string("feature-") + name] =
                     optional == 1 ? "0" : "1";
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index bdd39c6..687b5a6 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -3001,6 +3001,8 @@
     }
 
     {
+        Mutex::Autolock _l(mLock);
+
         if (!EffectsFactoryHalInterface::isNullUuid(&pDesc->uuid)) {
             // if uuid is specified, request effect descriptor
             lStatus = mEffectsFactoryHal->getDescriptor(&pDesc->uuid, &desc);
@@ -3056,6 +3058,8 @@
                 desc = d;
             }
         }
+    }
+    {
 
         // Do not allow auxiliary effects on a session different from 0 (output mix)
         if (sessionId != AUDIO_SESSION_OUTPUT_MIX &&
diff --git a/services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp b/services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp
index 84f0a89..70341c5 100644
--- a/services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp
+++ b/services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp
@@ -547,9 +547,11 @@
         return DONE;
     }
 
-    if (l.mParameters.isDeviceZslSupported) {
+    if ((l.mParameters.isDeviceZslSupported) && (l.mParameters.state != Parameters::RECORD) &&
+            (l.mParameters.state != Parameters::VIDEO_SNAPSHOT)) {
         // If device ZSL is supported, drop all pending preview buffers to reduce the chance of
         // rendering preview frames newer than the still frame.
+        // Additionally, preview must not get interrupted during video recording.
         client->getCameraDevice()->dropStreamBuffers(true, client->getPreviewStreamId());
     }