diff --git a/media/libstagefright/id3/ID3.cpp b/media/libstagefright/id3/ID3.cpp index 425468f41f..e97f6ebef4 100644 --- a/media/libstagefright/id3/ID3.cpp +++ b/media/libstagefright/id3/ID3.cpp @@ -107,20 +107,6 @@ ID3::ID3(DataSourceHelper *sourcehelper, bool ignoreV1, off64_t offset) } } -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 @@ struct id3_header { 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 @@ struct id3_header { 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 @@ struct id3_header { 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 @@ static void WriteSyncsafeInteger(uint8_t *dst, size_t x) { 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 @@ bool ID3::removeUnsynchronizationV2_4(bool iTunesHack) { } 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 a8f14706f6..cd5cd9e653 100644 --- a/media/libstagefright/id3/test/ID3Test.cpp +++ b/media/libstagefright/id3/test/ID3Test.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include "ID3TestEnvironment.h" @@ -42,7 +43,8 @@ TEST_P(ID3tagTest, TagTest) { string path = gEnv->getRes() + GetParam(); sp 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 @@ TEST_P(ID3versionTest, VersionTest) { sp 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 @@ TEST_P(ID3textTagTest, TextTagTest) { sp 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 @@ TEST_P(ID3albumArtTest, AlbumArtTest) { sp 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 @@ TEST_P(ID3multiAlbumArtTest, MultiAlbumArtTest) { sp 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 9984d853aa..5cd51cf71b 100644 --- a/media/libstagefright/id3/testid3.cpp +++ b/media/libstagefright/id3/testid3.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #define MAXPATHLEN 256 @@ -72,7 +73,8 @@ void scanFile(const char *path) { sp 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 2843a7a39c..0be5896aba 100644 --- a/media/libstagefright/include/ID3.h +++ b/media/libstagefright/include/ID3.h @@ -37,7 +37,6 @@ struct ID3 { }; 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();