Merge changes Ib293173b,I9244baf7 into rvc-dev
* changes:
Fix for potential NULL de-reference issue in Reverb
use vector::erase with std::remove_if
diff --git a/media/libstagefright/id3/ID3.cpp b/media/libstagefright/id3/ID3.cpp
index 425468f..e97f6eb 100644
--- a/media/libstagefright/id3/ID3.cpp
+++ b/media/libstagefright/id3/ID3.cpp
@@ -107,20 +107,6 @@
}
}
-ID3::ID3(DataSourceBase *source, bool ignoreV1, off64_t offset)
- : mIsValid(false),
- mData(NULL),
- mSize(0),
- mFirstFrameOffset(0),
- mVersion(ID3_UNKNOWN),
- mRawSize(0) {
- mIsValid = parseV2(source, offset);
-
- if (!mIsValid && !ignoreV1) {
- mIsValid = parseV1(source);
- }
-}
-
ID3::ID3(const uint8_t *data, size_t size, bool ignoreV1)
: mIsValid(false),
mData(NULL),
@@ -247,44 +233,14 @@
return false;
}
- if (header.version_major == 4) {
- void *copy = malloc(size);
- if (copy == NULL) {
- free(mData);
- mData = NULL;
- ALOGE("b/24623447, no more memory");
- return false;
- }
-
- memcpy(copy, mData, size);
-
- bool success = removeUnsynchronizationV2_4(false /* iTunesHack */);
- if (!success) {
- memcpy(mData, copy, size);
- mSize = size;
-
- success = removeUnsynchronizationV2_4(true /* iTunesHack */);
-
- if (success) {
- ALOGV("Had to apply the iTunes hack to parse this ID3 tag");
- }
- }
-
- free(copy);
- copy = NULL;
-
- if (!success) {
- free(mData);
- mData = NULL;
-
- return false;
- }
- } else if (header.flags & 0x80) {
+ // first handle global unsynchronization
+ if (header.flags & 0x80) {
ALOGV("removing unsynchronization");
removeUnsynchronization();
}
+ // handle extended header, if present
mFirstFrameOffset = 0;
if (header.version_major == 3 && (header.flags & 0x40)) {
// Version 2.3 has an optional extended header.
@@ -296,6 +252,7 @@
return false;
}
+ // v2.3 does not have syncsafe integers
size_t extendedHeaderSize = U32_AT(&mData[0]);
if (extendedHeaderSize > SIZE_MAX - 4) {
free(mData);
@@ -367,6 +324,48 @@
mFirstFrameOffset = ext_size;
}
+ // Handle any v2.4 per-frame unsynchronization
+ // The id3 spec isn't clear about what should happen if the global
+ // unsynchronization flag is combined with per-frame unsynchronization,
+ // or whether that's even allowed, so this code assumes id3 writing
+ // tools do the right thing and not apply double-unsynchronization,
+ // but will honor the flags if they are set.
+ if (header.version_major == 4) {
+ void *copy = malloc(size);
+ if (copy == NULL) {
+ free(mData);
+ mData = NULL;
+ ALOGE("b/24623447, no more memory");
+ return false;
+ }
+
+ memcpy(copy, mData, size);
+
+ bool success = removeUnsynchronizationV2_4(false /* iTunesHack */);
+ if (!success) {
+ memcpy(mData, copy, size);
+ mSize = size;
+
+ success = removeUnsynchronizationV2_4(true /* iTunesHack */);
+
+ if (success) {
+ ALOGV("Had to apply the iTunes hack to parse this ID3 tag");
+ }
+ }
+
+ free(copy);
+ copy = NULL;
+
+ if (!success) {
+ free(mData);
+ mData = NULL;
+
+ return false;
+ }
+ }
+
+
+
if (header.version_major == 2) {
mVersion = ID3_V2_2;
} else if (header.version_major == 3) {
@@ -411,7 +410,7 @@
bool ID3::removeUnsynchronizationV2_4(bool iTunesHack) {
size_t oldSize = mSize;
- size_t offset = 0;
+ size_t offset = mFirstFrameOffset;
while (mSize >= 10 && offset <= mSize - 10) {
if (!memcmp(&mData[offset], "\0\0\0\0", 4)) {
break;
@@ -445,7 +444,7 @@
}
if ((flags & 2) && (dataSize >= 2)) {
- // This file has "unsynchronization", so we have to replace occurrences
+ // This frame has "unsynchronization", so we have to replace occurrences
// of 0xff 0x00 with just 0xff in order to get the real data.
size_t readOffset = offset + 11;
diff --git a/media/libstagefright/id3/test/ID3Test.cpp b/media/libstagefright/id3/test/ID3Test.cpp
index a8f1470..cd5cd9e 100644
--- a/media/libstagefright/id3/test/ID3Test.cpp
+++ b/media/libstagefright/id3/test/ID3Test.cpp
@@ -24,6 +24,7 @@
#include <datasource/FileSource.h>
#include <media/stagefright/foundation/hexdump.h>
+#include <media/MediaExtractorPluginHelper.h>
#include <ID3.h>
#include "ID3TestEnvironment.h"
@@ -42,7 +43,8 @@
string path = gEnv->getRes() + GetParam();
sp<FileSource> file = new FileSource(path.c_str());
ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
- ID3 tag(file.get());
+ DataSourceHelper helper(file->wrap());
+ ID3 tag(&helper);
ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
ID3::Iterator it(tag, nullptr);
@@ -61,7 +63,8 @@
sp<android::FileSource> file = new FileSource(path.c_str());
ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
- ID3 tag(file.get());
+ DataSourceHelper helper(file->wrap());
+ ID3 tag(&helper);
ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
ASSERT_TRUE(tag.version() >= versionNumber)
<< "Expected version: " << tag.version() << " Found version: " << versionNumber;
@@ -73,7 +76,8 @@
sp<android::FileSource> file = new FileSource(path.c_str());
ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
- ID3 tag(file.get());
+ DataSourceHelper helper(file->wrap());
+ ID3 tag(&helper);
ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
int countTextFrames = 0;
ID3::Iterator it(tag, nullptr);
@@ -99,7 +103,8 @@
sp<android::FileSource> file = new FileSource(path.c_str());
ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
- ID3 tag(file.get());
+ DataSourceHelper helper(file->wrap());
+ ID3 tag(&helper);
ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
size_t dataSize;
String8 mime;
@@ -124,7 +129,8 @@
sp<android::FileSource> file = new FileSource(path.c_str());
ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
- ID3 tag(file.get());
+ DataSourceHelper helper(file->wrap());
+ ID3 tag(&helper);
ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
int count = 0;
ID3::Iterator it(tag, nullptr);
diff --git a/media/libstagefright/id3/testid3.cpp b/media/libstagefright/id3/testid3.cpp
index 9984d85..5cd51cf 100644
--- a/media/libstagefright/id3/testid3.cpp
+++ b/media/libstagefright/id3/testid3.cpp
@@ -24,6 +24,7 @@
#include <binder/ProcessState.h>
#include <datasource/FileSource.h>
#include <media/stagefright/foundation/ADebug.h>
+#include <media/MediaExtractorPluginHelper.h>
#define MAXPATHLEN 256
@@ -72,7 +73,8 @@
sp<FileSource> file = new FileSource(path);
CHECK_EQ(file->initCheck(), (status_t)OK);
- ID3 tag(file.get());
+ DataSourceHelper helper(file->wrap());
+ ID3 tag(&helper);
if (!tag.isValid()) {
printf("FAIL %s\n", path);
} else {
diff --git a/media/libstagefright/include/ID3.h b/media/libstagefright/include/ID3.h
index 2843a7a..0be5896 100644
--- a/media/libstagefright/include/ID3.h
+++ b/media/libstagefright/include/ID3.h
@@ -37,7 +37,6 @@
};
explicit ID3(DataSourceHelper *source, bool ignoreV1 = false, off64_t offset = 0);
- explicit ID3(DataSourceBase *source, bool ignoreV1 = false, off64_t offset = 0);
ID3(const uint8_t *data, size_t size, bool ignoreV1 = false);
~ID3();
diff --git a/media/libstagefright/tests/extractorFactory/Android.bp b/media/libstagefright/tests/extractorFactory/Android.bp
new file mode 100644
index 0000000..e3e61d7
--- /dev/null
+++ b/media/libstagefright/tests/extractorFactory/Android.bp
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+cc_test {
+ name: "ExtractorFactoryTest",
+ gtest: true,
+
+ srcs: [
+ "ExtractorFactoryTest.cpp",
+ ],
+
+ shared_libs: [
+ "liblog",
+ "libbase",
+ "libutils",
+ "libmedia",
+ "libbinder",
+ "libcutils",
+ "libdl_android",
+ "libdatasource",
+ "libmediametrics",
+ ],
+
+ static_libs: [
+ "libstagefright",
+ "libstagefright_foundation",
+ ],
+
+ include_dirs: [
+ "frameworks/av/media/libstagefright",
+ ],
+
+ // TODO: (b/150181583)
+ compile_multilib: "first",
+
+ cflags: [
+ "-Werror",
+ "-Wall",
+ ],
+
+ sanitize: {
+ cfi: true,
+ misc_undefined: [
+ "unsigned-integer-overflow",
+ "signed-integer-overflow",
+ ],
+ },
+}
diff --git a/media/libstagefright/tests/extractorFactory/AndroidTest.xml b/media/libstagefright/tests/extractorFactory/AndroidTest.xml
new file mode 100644
index 0000000..3aa6392
--- /dev/null
+++ b/media/libstagefright/tests/extractorFactory/AndroidTest.xml
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2020 The Android Open Source Project
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<configuration description="Test module config for extractor factory unit tests">
+ <option name="test-suite-tag" value="ExtractorFactoryTest" />
+ <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
+ <option name="cleanup" value="true" />
+ <option name="push" value="ExtractorFactoryTest->/data/local/tmp/ExtractorFactoryTest" />
+ <option name="push-file"
+ key="https://storage.googleapis.com/android_media/frameworks/av/media/extractors/tests/extractor.zip?unzip=true"
+ value="/data/local/tmp/ExtractorFactoryTestRes/" />
+ </target_preparer>
+
+ <test class="com.android.tradefed.testtype.GTest" >
+ <option name="native-test-device-path" value="/data/local/tmp" />
+ <option name="module-name" value="ExtractorFactoryTest" />
+ <option name="native-test-flag" value="-P /data/local/tmp/ExtractorFactoryTestRes/" />
+ </test>
+</configuration>
diff --git a/media/libstagefright/tests/extractorFactory/ExtractorFactoryTest.cpp b/media/libstagefright/tests/extractorFactory/ExtractorFactoryTest.cpp
new file mode 100644
index 0000000..d155caa
--- /dev/null
+++ b/media/libstagefright/tests/extractorFactory/ExtractorFactoryTest.cpp
@@ -0,0 +1,167 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+//#define LOG_NDEBUG 0
+#define LOG_TAG "ExtractorFactoryTest"
+#include <utils/Log.h>
+
+#include <binder/ProcessState.h>
+
+#include <datasource/FileSource.h>
+#include <media/stagefright/MediaExtractorFactory.h>
+#include <media/stagefright/foundation/MediaDefs.h>
+
+#include "ExtractorFactoryTestEnvironment.h"
+
+#define OUTPUT_FILE_NAME "/data/local/tmp/exFactoryLogs"
+
+using namespace android;
+
+static ExtractorFactoryTestEnvironment *gEnv = nullptr;
+
+class ExtractorFactoryTest : public ::testing::TestWithParam<pair<string, string>> {
+ public:
+ ExtractorFactoryTest() : mDataSource(nullptr), mExtractor(nullptr) {}
+
+ ~ExtractorFactoryTest() {
+ if (mDataSource) {
+ mDataSource.clear();
+ mDataSource = nullptr;
+ }
+ if (mExtractor) {
+ mExtractor.clear();
+ mExtractor = nullptr;
+ }
+ }
+
+ int32_t createDataSource(string inputFileName);
+ int32_t createExtractor(bool createFromService, string inputMime);
+
+ sp<DataSource> mDataSource;
+ sp<IMediaExtractor> mExtractor;
+};
+
+int32_t ExtractorFactoryTest::createDataSource(string inputFileName) {
+ FILE *mInputFp = fopen(inputFileName.c_str(), "rb");
+ if (!mInputFp) {
+ ALOGE("Unable to open input file : %s for reading", inputFileName.c_str());
+ return -1;
+ }
+ struct stat buf;
+ int32_t status = stat(inputFileName.c_str(), &buf);
+ if (status != 0) {
+ ALOGE("Failed to read file properties for input file : %s", inputFileName.c_str());
+ return -1;
+ }
+ int32_t fd = fileno(mInputFp);
+ if (fd < 0) {
+ ALOGE("Invalid file descriptor for input file : %s", inputFileName.c_str());
+ return -1;
+ }
+ mDataSource = new FileSource(dup(fd), 0, buf.st_size);
+ if (!mDataSource) return -1;
+ return 0;
+}
+
+int32_t ExtractorFactoryTest::createExtractor(bool createFromService, string inputMime) {
+ ALOGV("Creating extractor for mime : %s", inputMime.c_str());
+ if (createFromService) {
+ mExtractor = MediaExtractorFactory::CreateFromService(mDataSource, inputMime.c_str());
+ } else {
+ mExtractor = MediaExtractorFactory::Create(mDataSource);
+ }
+ if (mExtractor == nullptr) return -1;
+ return 0;
+}
+
+TEST_F(ExtractorFactoryTest, ListExtractorsTest) {
+ MediaExtractorFactory::LoadExtractors();
+ vector<std::string> supportedTypes = MediaExtractorFactory::getSupportedTypes();
+ ASSERT_GT(supportedTypes.size(), 0) << " MediaExtractorFactory doesn't suuport any extractor";
+
+ FILE *outputLog = fopen(OUTPUT_FILE_NAME, "wb");
+ ASSERT_NE(outputLog, nullptr) << "Unable to open output file - " << OUTPUT_FILE_NAME
+ << " for writing";
+
+ int32_t fd = fileno(outputLog);
+ ASSERT_GE(fd, 0);
+
+ Vector<String16> args;
+ int32_t status = MediaExtractorFactory::dump(fd, args);
+ ASSERT_EQ(status, OK) << "MediaExtractorFactory dump failed";
+ fclose(outputLog);
+}
+
+TEST_P(ExtractorFactoryTest, ExtractorFactoryApiTest) {
+ string inputMime = GetParam().second;
+ string inputFileName = gEnv->getRes() + GetParam().first;
+
+ MediaExtractorFactory::LoadExtractors();
+ bool createMode[] = {true, false};
+ for (bool createFromService : createMode) {
+ int32_t status = createDataSource(inputFileName);
+ ASSERT_EQ(status, 0) << "create data source failed";
+
+ status = createExtractor(createFromService, inputMime);
+ ASSERT_EQ(status, 0) << "Extractor creation failed for input: " << inputFileName;
+
+ int32_t numTracks = mExtractor->countTracks();
+ ASSERT_GT(numTracks, 0) << "Extractor didn't find any track for the given clip";
+
+ sp<MetaData> meta = mExtractor->getMetaData();
+ ASSERT_NE(meta, nullptr) << "getMetaData returned null";
+
+ const char *mime;
+ bool valueFound = meta->findCString(kKeyMIMEType, &mime);
+ ASSERT_TRUE(valueFound) << "Extractor did not provide MIME type";
+ ASSERT_EQ(mime, inputMime) << "Extractor factory returned invalid mime type";
+ mExtractor.clear();
+ mDataSource.clear();
+ }
+}
+
+// TODO: (b/150111966)
+// Replace mime strings with appropriate definitions
+INSTANTIATE_TEST_SUITE_P(
+ ExtractorFactoryTestAll, ExtractorFactoryTest,
+ ::testing::Values(make_pair("loudsoftaac.aac", MEDIA_MIMETYPE_AUDIO_AAC_ADTS),
+ make_pair("testamr.amr", "audio/amr"),
+ make_pair("amrwb.wav", MEDIA_MIMETYPE_AUDIO_AMR_WB),
+ make_pair("john_cage.ogg", MEDIA_MIMETYPE_CONTAINER_OGG),
+ make_pair("monotestgsm.wav", MEDIA_MIMETYPE_CONTAINER_WAV),
+ make_pair("segment000001.ts", MEDIA_MIMETYPE_CONTAINER_MPEG2TS),
+ make_pair("sinesweepflac.flac", MEDIA_MIMETYPE_AUDIO_FLAC),
+ make_pair("testopus.opus", MEDIA_MIMETYPE_CONTAINER_OGG),
+ make_pair("midi_a.mid", MEDIA_MIMETYPE_AUDIO_MIDI),
+ make_pair("sinesweepvorbis.mkv", MEDIA_MIMETYPE_CONTAINER_MATROSKA),
+ make_pair("sinesweepoggmp4.mp4", "audio/mp4"),
+ make_pair("sinesweepmp3lame.mp3", MEDIA_MIMETYPE_AUDIO_MPEG),
+ make_pair("swirl_144x136_vp9.webm", "video/webm"),
+ make_pair("swirl_144x136_vp8.webm", "video/webm"),
+ make_pair("swirl_132x130_mpeg4.mp4", MEDIA_MIMETYPE_CONTAINER_MPEG4)));
+
+int main(int argc, char **argv) {
+ ProcessState::self()->startThreadPool();
+ gEnv = new ExtractorFactoryTestEnvironment();
+ ::testing::AddGlobalTestEnvironment(gEnv);
+ ::testing::InitGoogleTest(&argc, argv);
+ int status = gEnv->initFromOptions(argc, argv);
+ if (status == 0) {
+ status = RUN_ALL_TESTS();
+ ALOGV("Test result = %d\n", status);
+ }
+ return status;
+}
diff --git a/media/libstagefright/tests/extractorFactory/ExtractorFactoryTestEnvironment.h b/media/libstagefright/tests/extractorFactory/ExtractorFactoryTestEnvironment.h
new file mode 100644
index 0000000..0fad4d3
--- /dev/null
+++ b/media/libstagefright/tests/extractorFactory/ExtractorFactoryTestEnvironment.h
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef __EXTRACTOR_FACTORY_TEST_ENVIRONMENT_H__
+#define __EXTRACTOR_FACTORY_TEST_ENVIRONMENT_H__
+
+#include <gtest/gtest.h>
+
+#include <getopt.h>
+
+using namespace std;
+
+class ExtractorFactoryTestEnvironment : public ::testing::Environment {
+ public:
+ ExtractorFactoryTestEnvironment() : res("/data/local/tmp/") {}
+
+ // Parses the command line arguments
+ int initFromOptions(int argc, char **argv);
+
+ void setRes(const char *_res) { res = _res; }
+
+ const string getRes() const { return res; }
+
+ private:
+ string res;
+};
+
+int ExtractorFactoryTestEnvironment::initFromOptions(int argc, char **argv) {
+ static struct option options[] = {{"res", required_argument, 0, 'P'}, {0, 0, 0, 0}};
+
+ while (true) {
+ int index = 0;
+ int c = getopt_long(argc, argv, "P:", options, &index);
+ if (c == -1) {
+ break;
+ }
+
+ switch (c) {
+ case 'P':
+ setRes(optarg);
+ break;
+ default:
+ break;
+ }
+ }
+
+ if (optind < argc) {
+ fprintf(stderr,
+ "unrecognized option: %s\n\n"
+ "usage: %s <gtest options> <test options>\n\n"
+ "test options are:\n\n"
+ "-P, --path: Resource files directory location\n",
+ argv[optind ?: 1], argv[0]);
+ return 2;
+ }
+ return 0;
+}
+
+#endif // __EXTRACTOR_FACTORY_TEST_ENVIRONMENT_H__
diff --git a/media/libstagefright/tests/extractorFactory/README.md b/media/libstagefright/tests/extractorFactory/README.md
new file mode 100644
index 0000000..aaa71aa
--- /dev/null
+++ b/media/libstagefright/tests/extractorFactory/README.md
@@ -0,0 +1,37 @@
+## Media Testing ##
+---
+#### Writer :
+The Writer Test Suite validates the writers available in libstagefright.
+
+Run the following steps to build the test suite:
+```
+mmm frameworks/av/media/libstagefright/tests/writer/
+```
+
+The 32-bit binaries will be created in the following path : ${OUT}/data/nativetest/
+The 64-bit binaries will be created in the following path : ${OUT}/data/nativetest64/
+
+To test 64-bit binary push binaries from nativetest64.
+
+adb push ${OUT}/data/nativetest64/ExtractorFactoryTest/ExtractorFactoryTest /data/local/tmp/
+
+To test 32-bit binary push binaries from nativetest.
+
+adb push ${OUT}/data/nativetest/ExtractorFactoryTest/ExtractorFactoryTest /data/local/tmp/
+
+The resource file for the tests is taken from [here](https://storage.googleapis.com/android_media/frameworks/av/media/extractors/tests/extractor.zip).
+Download, unzip and push these files into device for testing.
+
+```
+adb push extractor /data/local/tmp/
+```
+
+usage: ExtractorFactoryTest -P \<path_to_res_folder\>
+```
+adb shell /data/local/tmp/ExtractorFactoryTest -P /data/local/tmp/extractor/
+```
+Alternatively, the test can also be run using atest command.
+
+```
+atest ExtractorFactoryTest -- --enable-module-dynamic-download=true
+```