Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f29e14c6 authored by Ray Essick's avatar Ray Essick
Browse files

Further ID3 parsing work

Incorporate learnings from additional test clips

Bug: 184113853
Test: atest ID3Test -- --enable-module-dynamic-download=true
Merged-In: I19d68b1ed45a890ca91772322262c3eed11c0ff2
Change-Id: Id99a1601c6f777ea1494b7861e7a58aaace5d4b2
parent a3b006c8
Loading
Loading
Loading
Loading
+20 −9
Original line number Original line Diff line number Diff line
@@ -236,11 +236,19 @@ struct id3_header {
    // first handle global unsynchronization
    // first handle global unsynchronization
    bool hasGlobalUnsync = false;
    bool hasGlobalUnsync = false;
    if (header.flags & 0x80) {
    if (header.flags & 0x80) {
        ALOGV("removing unsynchronization");
        ALOGV("has Global unsynchronization");

        hasGlobalUnsync = true;
        hasGlobalUnsync = true;
        // we have to wait on applying global unsynchronization to V2.4 frames
        // if we apply it now, the length information within any V2.4 frames goes bad
        // Removing unsynchronization shrinks the buffer, but lengths (stored in safesync
        // format) stored within the frame reflect "pre-shrinking" totals.

        // we can (and should) apply the non-2.4 synch now.
        if ( header.version_major != 4) {
            ALOGV("Apply global unsync for non V2.4 frames");
            removeUnsynchronization();
            removeUnsynchronization();
        }
        }
    }


    // handle extended header, if present
    // handle extended header, if present
    mFirstFrameOffset = 0;
    mFirstFrameOffset = 0;
@@ -329,9 +337,10 @@ struct id3_header {
    // Handle any v2.4 per-frame unsynchronization
    // Handle any v2.4 per-frame unsynchronization
    // The id3 spec isn't clear about what should happen if the global
    // The id3 spec isn't clear about what should happen if the global
    // unsynchronization flag is combined with per-frame unsynchronization,
    // unsynchronization flag is combined with per-frame unsynchronization,
    // or whether that's even allowed, so this code assumes id3 writing
    // or whether that's even allowed. We choose a "no more than 1 unsynchronization"
    // tools do the right thing and not apply double-unsynchronization,
    // semantic; the V2_4 unsynchronizer gets a copy of the global flag so it can handle
    // but will honor the flags if they are set.
    // this possible ambiquity.
    //
    if (header.version_major == 4) {
    if (header.version_major == 4) {
        void *copy = malloc(size);
        void *copy = malloc(size);
        if (copy == NULL) {
        if (copy == NULL) {
@@ -367,7 +376,6 @@ struct id3_header {
    }
    }





    if (header.version_major == 2) {
    if (header.version_major == 2) {
        mVersion = ID3_V2_2;
        mVersion = ID3_V2_2;
    } else if (header.version_major == 3) {
    } else if (header.version_major == 3) {
@@ -445,7 +453,11 @@ bool ID3::removeUnsynchronizationV2_4(bool iTunesHack, bool hasGlobalUnsync) {
            flags &= ~1;
            flags &= ~1;
        }
        }


        if (!hasGlobalUnsync && (flags & 2) && (dataSize >= 2)) {
        ALOGV("hasglobal %d  flags&2 %d", hasGlobalUnsync, flags&2);
        if (hasGlobalUnsync && !(flags & 2)) {
            ALOGV("OOPS: global unsync set, but per-frame NOT set; removing unsync anyway");
        }
        if ((hasGlobalUnsync || (flags & 2)) && (dataSize >= 2)) {
            // This frame 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.
            // of 0xff 0x00 with just 0xff in order to get the real data.


@@ -472,7 +484,6 @@ bool ID3::removeUnsynchronizationV2_4(bool iTunesHack, bool hasGlobalUnsync) {
                ALOGE("b/34618607 (%zu %zu %zu %zu)", readOffset, writeOffset, oldSize, mSize);
                ALOGE("b/34618607 (%zu %zu %zu %zu)", readOffset, writeOffset, oldSize, mSize);
                android_errorWriteLog(0x534e4554, "34618607");
                android_errorWriteLog(0x534e4554, "34618607");
            }
            }

        }
        }
        flags &= ~2;
        flags &= ~2;
        if (flags != prevFlags || iTunesHack) {
        if (flags != prevFlags || iTunesHack) {
+1 −1
Original line number Original line Diff line number Diff line
@@ -19,7 +19,7 @@
        <option name="cleanup" value="true" />
        <option name="cleanup" value="true" />
        <option name="push" value="ID3Test->/data/local/tmp/ID3Test" />
        <option name="push" value="ID3Test->/data/local/tmp/ID3Test" />
        <option name="push-file"
        <option name="push-file"
            key="https://storage.googleapis.com/android_media/frameworks/av/media/libstagefright/id3/test/ID3Test.zip?unzip=true"
            key="https://storage.googleapis.com/android_media/frameworks/av/media/libstagefright/id3/test/ID3Test-1.2.zip?unzip=true"
            value="/data/local/tmp/ID3TestRes/" />
            value="/data/local/tmp/ID3TestRes/" />
    </target_preparer>
    </target_preparer>


+112 −56
Original line number Original line Diff line number Diff line
@@ -29,6 +29,7 @@


#include "ID3TestEnvironment.h"
#include "ID3TestEnvironment.h"



using namespace android;
using namespace android;


static ID3TestEnvironment *gEnv = nullptr;
static ID3TestEnvironment *gEnv = nullptr;
@@ -41,6 +42,7 @@ class ID3multiAlbumArtTest : public ::testing::TestWithParam<pair<string, int>>


TEST_P(ID3tagTest, TagTest) {
TEST_P(ID3tagTest, TagTest) {
    string path = gEnv->getRes() + GetParam();
    string path = gEnv->getRes() + GetParam();
    ALOGV(" =====   TagTest for %s", path.c_str());
    sp<FileSource> file = new FileSource(path.c_str());
    sp<FileSource> file = new FileSource(path.c_str());
    ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
    ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
    DataSourceHelper helper(file->wrap());
    DataSourceHelper helper(file->wrap());
@@ -51,7 +53,7 @@ TEST_P(ID3tagTest, TagTest) {
    while (!it.done()) {
    while (!it.done()) {
        String8 id;
        String8 id;
        it.getID(&id);
        it.getID(&id);
        ASSERT_GT(id.length(), 0) << "No ID tag found! \n";
        ASSERT_GT(id.length(), 0) << "Found an ID3 tag of 0 size";
        ALOGV("Found ID tag: %s\n", String8(id).c_str());
        ALOGV("Found ID tag: %s\n", String8(id).c_str());
        it.next();
        it.next();
    }
    }
@@ -60,19 +62,21 @@ TEST_P(ID3tagTest, TagTest) {
TEST_P(ID3versionTest, VersionTest) {
TEST_P(ID3versionTest, VersionTest) {
    int versionNumber = GetParam().second;
    int versionNumber = GetParam().second;
    string path = gEnv->getRes() + GetParam().first;
    string path = gEnv->getRes() + GetParam().first;
    ALOGV(" =====   VersionTest for %s", path.c_str());
    sp<android::FileSource> file = new FileSource(path.c_str());
    sp<android::FileSource> file = new FileSource(path.c_str());
    ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
    ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";


    DataSourceHelper helper(file->wrap());
    DataSourceHelper helper(file->wrap());
    ID3 tag(&helper);
    ID3 tag(&helper);
    ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
    ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
    ASSERT_TRUE(tag.version() >= versionNumber)
    ASSERT_EQ(tag.version(), versionNumber)
            << "Expected version: " << tag.version() << " Found version: " << versionNumber;
            << "Found version: " << tag.version() << " Expected version: " << versionNumber;
}
}


TEST_P(ID3textTagTest, TextTagTest) {
TEST_P(ID3textTagTest, TextTagTest) {
    int numTextFrames = GetParam().second;
    int numTextFrames = GetParam().second;
    string path = gEnv->getRes() + GetParam().first;
    string path = gEnv->getRes() + GetParam().first;
    ALOGV(" =====   TextTagTest for %s", path.c_str());
    sp<android::FileSource> file = new FileSource(path.c_str());
    sp<android::FileSource> file = new FileSource(path.c_str());
    ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
    ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";


@@ -81,10 +85,11 @@ TEST_P(ID3textTagTest, TextTagTest) {
    ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
    ASSERT_TRUE(tag.isValid()) << "No valid ID3 tag found for " << path.c_str() << "\n";
    int countTextFrames = 0;
    int countTextFrames = 0;
    ID3::Iterator it(tag, nullptr);
    ID3::Iterator it(tag, nullptr);
    if (tag.version() != ID3::ID3_V1 && tag.version() != ID3::ID3_V1_1) {
        while (!it.done()) {
        while (!it.done()) {
            String8 id;
            String8 id;
            it.getID(&id);
            it.getID(&id);
        ASSERT_GT(id.length(), 0);
            ASSERT_GT(id.length(), 0) << "Found an ID3 tag of 0 size";
            if (id[0] == 'T') {
            if (id[0] == 'T') {
                String8 text;
                String8 text;
                countTextFrames++;
                countTextFrames++;
@@ -93,6 +98,22 @@ TEST_P(ID3textTagTest, TextTagTest) {
            }
            }
            it.next();
            it.next();
        }
        }
    } else {
        while (!it.done()) {
            String8 id;
            String8 text;
            it.getID(&id);
            ASSERT_GT(id.length(), 0) << "Found an ID3 tag of 0 size";
            it.getString(&text);
            // if the tag has a value
            if (strcmp(text.string(), "")) {
                countTextFrames++;
                ALOGV("ID: %s\n", id.c_str());
                ALOGV("Text string: %s\n", text.string());
            }
            it.next();
        }
    }
    ASSERT_EQ(countTextFrames, numTextFrames)
    ASSERT_EQ(countTextFrames, numTextFrames)
            << "Expected " << numTextFrames << " text frames, found " << countTextFrames;
            << "Expected " << numTextFrames << " text frames, found " << countTextFrames;
}
}
@@ -100,6 +121,7 @@ TEST_P(ID3textTagTest, TextTagTest) {
TEST_P(ID3albumArtTest, AlbumArtTest) {
TEST_P(ID3albumArtTest, AlbumArtTest) {
    bool albumArtPresent = GetParam().second;
    bool albumArtPresent = GetParam().second;
    string path = gEnv->getRes() + GetParam().first;
    string path = gEnv->getRes() + GetParam().first;
    ALOGV(" =====   AlbumArt for %s", path.c_str());
    sp<android::FileSource> file = new FileSource(path.c_str());
    sp<android::FileSource> file = new FileSource(path.c_str());
    ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";
    ASSERT_EQ(file->initCheck(), (status_t)OK) << "File initialization failed! \n";


@@ -118,6 +140,7 @@ TEST_P(ID3albumArtTest, AlbumArtTest) {
    } else {
    } else {
        ASSERT_EQ(data, nullptr) << "Found album art when expected none!";
        ASSERT_EQ(data, nullptr) << "Found album art when expected none!";
    }
    }

#if (LOG_NDEBUG == 0)
#if (LOG_NDEBUG == 0)
    hexdump(data, dataSize > 128 ? 128 : dataSize);
    hexdump(data, dataSize > 128 ? 128 : dataSize);
#endif
#endif
@@ -137,7 +160,7 @@ TEST_P(ID3multiAlbumArtTest, MultiAlbumArtTest) {
    while (!it.done()) {
    while (!it.done()) {
        String8 id;
        String8 id;
        it.getID(&id);
        it.getID(&id);
        ASSERT_GT(id.length(), 0);
        ASSERT_GT(id.length(), 0) << "Found an ID3 tag of 0 size";
        // Check if the tag is an "APIC/PIC" tag.
        // Check if the tag is an "APIC/PIC" tag.
        if (String8(id) == "APIC" || String8(id) == "PIC") {
        if (String8(id) == "APIC" || String8(id) == "PIC") {
            count++;
            count++;
@@ -158,57 +181,90 @@ TEST_P(ID3multiAlbumArtTest, MultiAlbumArtTest) {
                                  << " album arts! \n";
                                  << " album arts! \n";
}
}


// we have a test asset with large album art -- which is larger than our 3M cap
// that we inserted intentionally in the ID3 parsing routine.
// Rather than have it fail all the time, we have wrapped it under an #ifdef
// so that the tests will pass.
#undef  TEST_LARGE


// it appears that bbb_2sec_v24_unsynchronizedAllFrames.mp3 is not a legal file,
// so we've commented it out of the list of files to be tested
//

INSTANTIATE_TEST_SUITE_P(id3TestAll, ID3tagTest,
INSTANTIATE_TEST_SUITE_P(id3TestAll, ID3tagTest,
                         ::testing::Values("bbb_44100hz_2ch_128kbps_mp3_30sec.mp3",
                         ::testing::Values("bbb_1sec_v23.mp3",
                                           "bbb_44100hz_2ch_128kbps_mp3_30sec_1_image.mp3",
                                           "bbb_1sec_1_image.mp3",
                                           "bbb_44100hz_2ch_128kbps_mp3_30sec_2_image.mp3",
                                           "bbb_1sec_2_image.mp3",
                                           "bbb_44100hz_2ch_128kbps_mp3_5mins.mp3",
                                           "bbb_2sec_v24.mp3",
                                           "bbb_44100hz_2ch_128kbps_mp3_5mins_1_image.mp3",
                                           "bbb_2sec_1_image.mp3",
                                           "bbb_44100hz_2ch_128kbps_mp3_5mins_2_image.mp3",
                                           "bbb_2sec_2_image.mp3",
                                           "bbb_44100hz_2ch_128kbps_mp3_5mins_largeSize.mp3",
                                           "bbb_2sec_largeSize.mp3",
                                           "bbb_44100hz_2ch_128kbps_mp3_30sec_moreTextFrames.mp3"));
                                           "bbb_1sec_v23_3tags.mp3",
                                           "bbb_1sec_v1_5tags.mp3",
                                           "bbb_2sec_v24_unsynchronizedOneFrame.mp3",
                                           "idv24_unsynchronized.mp3"));


INSTANTIATE_TEST_SUITE_P(
INSTANTIATE_TEST_SUITE_P(
        id3TestAll, ID3versionTest,
        id3TestAll, ID3versionTest,
        ::testing::Values(make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec.mp3", 4),
        ::testing::Values(make_pair("bbb_1sec_v23.mp3", ID3::ID3_V2_3),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_1_image.mp3", 4),
                          make_pair("bbb_1sec_1_image.mp3", ID3::ID3_V2_3),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_2_image.mp3", 4),
                          make_pair("bbb_1sec_2_image.mp3", ID3::ID3_V2_3),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins.mp3", 4),
                          make_pair("bbb_2sec_v24.mp3", ID3::ID3_V2_4),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_1_image.mp3", 4),
                          make_pair("bbb_2sec_1_image.mp3", ID3::ID3_V2_4),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_2_image.mp3", 4),
                          make_pair("bbb_2sec_2_image.mp3", ID3::ID3_V2_4),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_largeSize.mp3", 4),
#if TEST_LARGE
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_moreTextFrames.mp3", 4)));
                          make_pair("bbb_2sec_largeSize.mp3", ID3::ID3_V2_4), // FAIL
#endif
                          make_pair("bbb_1sec_v23_3tags.mp3", ID3::ID3_V2_3),
                          make_pair("bbb_1sec_v1_5tags.mp3", ID3::ID3_V1_1),
                          make_pair("bbb_1sec_v1_3tags.mp3", ID3::ID3_V1_1),
                          make_pair("bbb_2sec_v24_unsynchronizedOneFrame.mp3", ID3::ID3_V2_4),
                          make_pair("idv24_unsynchronized.mp3", ID3::ID3_V2_4)));


INSTANTIATE_TEST_SUITE_P(
INSTANTIATE_TEST_SUITE_P(
        id3TestAll, ID3textTagTest,
        id3TestAll, ID3textTagTest,
        ::testing::Values(make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec.mp3", 1),
        ::testing::Values(
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_1_image.mp3", 1),
                make_pair("bbb_1sec_v23.mp3", 1),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_2_image.mp3", 1),
                make_pair("bbb_1sec_1_image.mp3", 1),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins.mp3", 1),
                make_pair("bbb_1sec_2_image.mp3", 1),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_1_image.mp3", 1),
                make_pair("bbb_2sec_v24.mp3", 1),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_2_image.mp3", 1),
                make_pair("bbb_2sec_1_image.mp3", 1),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_largeSize.mp3", 1),
                make_pair("bbb_2sec_2_image.mp3", 1),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_moreTextFrames.mp3", 5)));
#if TEST_LARGE
                make_pair("bbb_2sec_largeSize.mp3", 1), // FAIL
#endif
                make_pair("bbb_1sec_v23_3tags.mp3", 3),
                make_pair("bbb_1sec_v1_5tags.mp3", 5),
                make_pair("bbb_1sec_v1_3tags.mp3", 3),
                make_pair("bbb_2sec_v24_unsynchronizedOneFrame.mp3", 3)
                ));


INSTANTIATE_TEST_SUITE_P(
INSTANTIATE_TEST_SUITE_P(id3TestAll, ID3albumArtTest,
        id3TestAll, ID3albumArtTest,
                         ::testing::Values(make_pair("bbb_1sec_v23.mp3", false),
        ::testing::Values(make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec.mp3", false),
                                           make_pair("bbb_1sec_1_image.mp3", true),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_1_image.mp3", true),
                                           make_pair("bbb_1sec_2_image.mp3", true),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_2_image.mp3", true),
                                           make_pair("bbb_2sec_v24.mp3", false),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins.mp3", false),
                                           make_pair("bbb_2sec_1_image.mp3", true),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_1_image.mp3", true),
                                           make_pair("bbb_2sec_2_image.mp3", true),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_2_image.mp3", true),
#if TEST_LARGE
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_largeSize.mp3", true)));
                                           make_pair("bbb_2sec_largeSize.mp3", true), // FAIL
#endif
                                           make_pair("bbb_1sec_v1_5tags.mp3", false),
                                           make_pair("idv24_unsynchronized.mp3", true)
                                           ));


INSTANTIATE_TEST_SUITE_P(
INSTANTIATE_TEST_SUITE_P(id3TestAll, ID3multiAlbumArtTest,
        id3TestAll, ID3multiAlbumArtTest,
                         ::testing::Values(make_pair("bbb_1sec_v23.mp3", 0),
        ::testing::Values(make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec.mp3", 0),
                                           make_pair("bbb_2sec_v24.mp3", 0),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins.mp3", 0),
#if TEST_LARGE
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_1_image.mp3", 1),
                                           make_pair("bbb_2sec_largeSize.mp3", 3), // FAIL
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_1_image.mp3", 1),
#endif
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_30sec_2_image.mp3", 2),
                                           make_pair("bbb_1sec_1_image.mp3", 1),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_2_image.mp3", 2),
                                           make_pair("bbb_2sec_1_image.mp3", 1),
                          make_pair("bbb_44100hz_2ch_128kbps_mp3_5mins_largeSize.mp3", 3)));
                                           make_pair("bbb_1sec_2_image.mp3", 2),
                                           make_pair("bbb_2sec_2_image.mp3", 2)
                                           ));


int main(int argc, char **argv) {
int main(int argc, char **argv) {
    gEnv = new ID3TestEnvironment();
    gEnv = new ID3TestEnvironment();