diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a29841e..ae30f7ad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,10 +42,10 @@ set(TESTS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/tests/") ## the following are directories where stuff will be installed to set(LIB_SUFFIX "" CACHE STRING "Define suffix of directory name (32/64)") -set(EXEC_INSTALL_PREFIX "${CMAKE_INSTALL_PREFIX}" CACHE PATH "Base directory for executables and libraries" FORCE) -set(BIN_INSTALL_DIR "${EXEC_INSTALL_PREFIX}/bin" CACHE PATH "The subdirectory to the binaries prefix (default prefix/bin)" FORCE) -set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}/lib${LIB_SUFFIX}" CACHE PATH "The subdirectory relative to the install prefix where libraries will be installed (default is /lib${LIB_SUFFIX})" FORCE) -set(INCLUDE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/include" CACHE PATH "The subdirectory to the header prefix" FORCE) +set(EXEC_INSTALL_PREFIX "${CMAKE_INSTALL_PREFIX}" CACHE PATH "Base directory for executables and libraries") +set(BIN_INSTALL_DIR "${EXEC_INSTALL_PREFIX}/bin" CACHE PATH "The subdirectory to the binaries prefix (default prefix/bin)") +set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}/lib${LIB_SUFFIX}" CACHE PATH "The subdirectory relative to the install prefix where libraries will be installed (default is /lib${LIB_SUFFIX})") +set(INCLUDE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/include" CACHE PATH "The subdirectory to the header prefix") if(APPLE) option(BUILD_FRAMEWORK "Build an OS X framework" OFF) @@ -110,7 +110,7 @@ if(WIN32) endif() if(NOT BUILD_FRAMEWORK) - configure_file("${CMAKE_CURRENT_SOURCE_DIR}/taglib.pc.cmake" "${CMAKE_CURRENT_BINARY_DIR}/taglib.pc") + configure_file("${CMAKE_CURRENT_SOURCE_DIR}/taglib.pc.cmake" "${CMAKE_CURRENT_BINARY_DIR}/taglib.pc" @ONLY) install(FILES "${CMAKE_CURRENT_BINARY_DIR}/taglib.pc" DESTINATION "${LIB_INSTALL_DIR}/pkgconfig") endif() diff --git a/NEWS b/NEWS index 89839d83..e442195b 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ * Added support for ID3v2 PCST and WFED frames. * Added support for pictures in XiphComment. * Added String::clear(). + * Added FLAC::File::strip() for removing non-standard tags. * Added alternative functions to XiphComment::removeField(). * Added BUILD_BINDINGS build option. * Added ENABLE_CCACHE build option. @@ -11,6 +12,7 @@ * Better handling of duplicate tag chunks in WAV files. * Better handling of duplicate tag chunks in AIFF files. * Better handling of duplicate Vorbis comment blocks in FLAC files. + * Better handling of broken MPEG audio frames. * Fixed crash when calling File::properties() after strip(). * Fixed crash when parsing certain MPEG files. * Fixed possible file corruptions when saving ASF files. diff --git a/taglib.pc.cmake b/taglib.pc.cmake index 09402998..ba9d655b 100644 --- a/taglib.pc.cmake +++ b/taglib.pc.cmake @@ -1,11 +1,11 @@ -prefix=${CMAKE_INSTALL_PREFIX} -exec_prefix=${CMAKE_INSTALL_PREFIX} -libdir=${LIB_INSTALL_DIR} -includedir=${INCLUDE_INSTALL_DIR} +prefix=@CMAKE_INSTALL_PREFIX@ +exec_prefix=@CMAKE_INSTALL_PREFIX@ +libdir=@LIB_INSTALL_DIR@ +includedir=@INCLUDE_INSTALL_DIR@ Name: TagLib Description: Audio meta-data library Requires: -Version: ${TAGLIB_LIB_VERSION_STRING} -Libs: -L${dollar}{libdir} -ltag -Cflags: -I${dollar}{includedir}/taglib +Version: @TAGLIB_LIB_VERSION_STRING@ +Libs: -L${libdir} -ltag +Cflags: -I${includedir}/taglib diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index 9d668b40..30114667 100644 --- a/taglib/flac/flacfile.cpp +++ b/taglib/flac/flacfile.cpp @@ -341,6 +341,20 @@ void FLAC::File::removePictures() } } +void FLAC::File::strip(int tags) +{ + if(tags & ID3v1) + d->tag.set(FlacID3v1Index, 0); + + if(tags & ID3v2) + d->tag.set(FlacID3v2Index, 0); + + if(tags & XiphComment) { + xiphComment()->removeAllFields(); + xiphComment()->removeAllPictures(); + } +} + bool FLAC::File::hasXiphComment() const { return !d->xiphCommentData.isEmpty(); diff --git a/taglib/flac/flacfile.h b/taglib/flac/flacfile.h index 31b6de67..30ecf4b1 100644 --- a/taglib/flac/flacfile.h +++ b/taglib/flac/flacfile.h @@ -66,6 +66,23 @@ namespace TagLib { class TAGLIB_EXPORT File : public TagLib::File { public: + /*! + * This set of flags is used for various operations and is suitable for + * being OR-ed together. + */ + enum TagTypes { + //! Empty set. Matches no tag types. + NoTags = 0x0000, + //! Matches Vorbis comments. + XiphComment = 0x0001, + //! Matches ID3v1 tags. + ID3v1 = 0x0002, + //! Matches ID3v2 tags. + ID3v2 = 0x0004, + //! Matches all tag types. + AllTags = 0xffff + }; + /*! * Constructs an FLAC file from \a file. If \a readProperties is true the * file's audio properties will also be read. @@ -227,6 +244,21 @@ namespace TagLib { */ void addPicture(Picture *picture); + /*! + * This will remove the tags that match the OR-ed together TagTypes from + * the file. By default it removes all tags. + * + * \warning This will also invalidate pointers to the tags as their memory + * will be freed. + * + * \note In order to make the removal permanent save() still needs to be + * called. + * + * \note This won't remove the Vorbis comment block completely. The + * vendor ID will be preserved. + */ + void strip(int tags = AllTags); + /*! * Returns whether or not the file on disk actually has a XiphComment. * diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 37acd020..2f21fe5a 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -650,10 +650,18 @@ ByteVector ID3v2::Tag::render() const void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const { +#ifdef NO_ITUNES_HACKS const char *unsupportedFrames[] = { "ASPI", "EQU2", "RVA2", "SEEK", "SIGN", "TDRL", "TDTG", "TMOO", "TPRO", "TSOA", "TSOT", "TSST", "TSOP", 0 }; +#else + // iTunes writes and reads TSOA, TSOT, TSOP to ID3v2.3. + const char *unsupportedFrames[] = { + "ASPI", "EQU2", "RVA2", "SEEK", "SIGN", "TDRL", "TDTG", + "TMOO", "TPRO", "TSST", 0 + }; +#endif ID3v2::TextIdentificationFrame *frameTDOR = 0; ID3v2::TextIdentificationFrame *frameTDRC = 0; ID3v2::TextIdentificationFrame *frameTIPL = 0; diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index ca2af56a..9c0f3451 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -34,29 +34,11 @@ #include "mpegfile.h" #include "mpegheader.h" +#include "mpegutils.h" #include "tpropertymap.h" using namespace TagLib; -namespace -{ - /*! - * MPEG frames can be recognized by the bit pattern 11111111 111, so the - * first byte is easy to check for, however checking to see if the second byte - * starts with \e 111 is a bit more tricky, hence these functions. - */ - - inline bool firstSyncByte(unsigned char byte) - { - return (byte == 0xFF); - } - - inline bool secondSynchByte(unsigned char byte) - { - return ((byte & 0xE0) == 0xE0); - } -} - namespace { enum { ID3v2Index = 0, APEIndex = 1, ID3v1Index = 2 }; diff --git a/taglib/mpeg/mpegheader.cpp b/taglib/mpeg/mpegheader.cpp index 9efb456b..e6c210ec 100644 --- a/taglib/mpeg/mpegheader.cpp +++ b/taglib/mpeg/mpegheader.cpp @@ -25,10 +25,12 @@ #include #include +#include #include #include #include "mpegheader.h" +#include "mpegutils.h" using namespace TagLib; @@ -79,7 +81,13 @@ public: MPEG::Header::Header(const ByteVector &data) : d(new HeaderPrivate()) { - parse(data); + debug("MPEG::Header::Header() - This constructor is no longer used."); +} + +MPEG::Header::Header(File *file, long long offset, bool checkLength) : + d(new HeaderPrivate()) +{ + parse(file, offset, checkLength); } MPEG::Header::Header(const Header &h) : @@ -162,8 +170,11 @@ MPEG::Header &MPEG::Header::operator=(const Header &h) // private members //////////////////////////////////////////////////////////////////////////////// -void MPEG::Header::parse(const ByteVector &data) +void MPEG::Header::parse(File *file, long long offset, bool checkLength) { + file->seek(offset); + const ByteVector data = file->readBlock(4); + if(data.size() < 4) { debug("MPEG::Header::parse() -- data is too short for an MPEG frame header."); return; @@ -171,13 +182,8 @@ void MPEG::Header::parse(const ByteVector &data) // Check for the MPEG synch bytes. - if(static_cast(data[0]) != 0xFF) { - debug("MPEG::Header::parse() -- First byte did not match MPEG synch."); - return; - } - - if((static_cast(data[1]) & 0xE0) != 0xE0) { - debug("MPEG::Header::parse() -- Second byte did not match MPEG synch."); + if(!firstSyncByte(data[0]) || !secondSynchByte(data[1])) { + debug("MPEG::Header::parse() -- MPEG header did not match MPEG synch."); return; } @@ -293,6 +299,31 @@ void MPEG::Header::parse(const ByteVector &data) if(d->data->isPadded) d->data->frameLength += paddingSize[layerIndex]; + // Check if the frame length has been calculated correctly, or the next frame + // synch bytes are right next to the end of this frame. + + // We read some extra bytes to be a bit tolerant. + + if(checkLength) { + + bool nextFrameFound = false; + + file->seek(offset + d->data->frameLength); + const ByteVector nextSynch = file->readBlock(4); + + for(int i = 0; i < static_cast(nextSynch.size()) - 1; ++i) { + if(firstSyncByte(nextSynch[i]) && secondSynchByte(nextSynch[i + 1])) { + nextFrameFound = true; + break; + } + } + + if(!nextFrameFound) { + debug("MPEG::Header::parse() -- Calculated frame length did not match the actual length."); + return; + } + } + // Now that we're done parsing, set this to be a valid frame. d->data->isValid = true; diff --git a/taglib/mpeg/mpegheader.h b/taglib/mpeg/mpegheader.h index a55cac09..3ab986ab 100644 --- a/taglib/mpeg/mpegheader.h +++ b/taglib/mpeg/mpegheader.h @@ -31,6 +31,7 @@ namespace TagLib { class ByteVector; + class File; namespace MPEG { @@ -48,9 +49,20 @@ namespace TagLib { public: /*! * Parses an MPEG header based on \a data. + * + * \deprecated */ Header(const ByteVector &data); + /*! + * Parses an MPEG header based on \a file and \a offset. + * + * \note If \a checkLength is true, this requires the next MPEG frame to + * check if the frame length is parsed and calculated correctly. So it's + * suitable for seeking for the first valid frame. + */ + Header(File *file, long long offset, bool checkLength = true); + /*! * Does a shallow copy of \a h. */ @@ -155,7 +167,7 @@ namespace TagLib { Header &operator=(const Header &h); private: - void parse(const ByteVector &data); + void parse(File *file, long long offset, bool checkLength); class HeaderPrivate; HeaderPrivate *d; diff --git a/taglib/mpeg/mpegproperties.cpp b/taglib/mpeg/mpegproperties.cpp index c58229af..1409ea19 100644 --- a/taglib/mpeg/mpegproperties.cpp +++ b/taglib/mpeg/mpegproperties.cpp @@ -158,8 +158,7 @@ void MPEG::AudioProperties::read(File *file) return; } - file->seek(firstFrameOffset); - Header firstHeader(file->readBlock(4)); + Header firstHeader(file, firstFrameOffset); while(!firstHeader.isValid()) { firstFrameOffset = file->nextFrameOffset(firstFrameOffset + 1); @@ -168,8 +167,7 @@ void MPEG::AudioProperties::read(File *file) return; } - file->seek(firstFrameOffset); - firstHeader = Header(file->readBlock(4)); + firstHeader = Header(file, firstFrameOffset); } // Check for a VBR header that will help us in gathering information about a @@ -200,14 +198,27 @@ void MPEG::AudioProperties::read(File *file) d->bitrate = firstHeader.bitrate(); - long long streamLength = file->length() - firstFrameOffset; + // Look for the last MPEG audio frame to calculate the stream length. - if(file->hasID3v1Tag()) - streamLength -= 128; + long long lastFrameOffset = file->lastFrameOffset(); + if(lastFrameOffset < 0) { + debug("MPEG::Properties::read() -- Could not find an MPEG frame in the stream."); + return; + } - if(file->hasAPETag()) - streamLength -= file->APETag()->footer()->completeTagSize(); + Header lastHeader(file, lastFrameOffset, false); + while(!lastHeader.isValid()) { + lastFrameOffset = file->previousFrameOffset(lastFrameOffset); + if(lastFrameOffset < 0) { + debug("MPEG::Properties::read() -- Could not find a valid last MPEG frame in the stream."); + return; + } + + lastHeader = Header(file, lastFrameOffset, false); + } + + const long long streamLength = lastFrameOffset - firstFrameOffset + lastHeader.frameLength(); if(streamLength > 0) d->length = static_cast(streamLength * 8.0 / d->bitrate + 0.5); } diff --git a/taglib/mpeg/mpegutils.h b/taglib/mpeg/mpegutils.h new file mode 100644 index 00000000..78cee280 --- /dev/null +++ b/taglib/mpeg/mpegutils.h @@ -0,0 +1,59 @@ +/*************************************************************************** + copyright : (C) 2015 by Tsuda Kageyu + email : tsuda.kageyu@gmail.com + ***************************************************************************/ + +/*************************************************************************** + * This library is free software; you can redistribute it and/or modify * + * it under the terms of the GNU Lesser General Public License version * + * 2.1 as published by the Free Software Foundation. * + * * + * This library is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with this library; if not, write to the Free Software * + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * + * 02110-1301 USA * + * * + * Alternatively, this file is available under the Mozilla Public * + * License Version 1.1. You may obtain a copy of the License at * + * http://www.mozilla.org/MPL/ * + ***************************************************************************/ + +#ifndef TAGLIB_MPEGUTILS_H +#define TAGLIB_MPEGUTILS_H + +// THIS FILE IS NOT A PART OF THE TAGLIB API + +#ifndef DO_NOT_DOCUMENT // tell Doxygen not to document this header + +namespace TagLib +{ + namespace MPEG + { + + /*! + * MPEG frames can be recognized by the bit pattern 11111111 111, so the + * first byte is easy to check for, however checking to see if the second byte + * starts with \e 111 is a bit more tricky, hence these functions. + */ + + inline bool firstSyncByte(unsigned char byte) + { + return (byte == 0xFF); + } + + inline bool secondSynchByte(unsigned char byte) + { + return ((byte & 0xE0) == 0xE0); + } + + } +} + +#endif + +#endif diff --git a/taglib/ogg/oggfile.cpp b/taglib/ogg/oggfile.cpp index 0a1a6b5c..47ff95f3 100644 --- a/taglib/ogg/oggfile.cpp +++ b/taglib/ogg/oggfile.cpp @@ -34,15 +34,24 @@ using namespace TagLib; +namespace +{ + // Returns the first packet index of the right next page to the given one. + inline unsigned int nextPacketIndex(const Ogg::Page *page) + { + if(page->header()->lastPacketCompleted()) + return page->firstPacketIndex() + page->packetCount(); + else + return page->firstPacketIndex() + page->packetCount() - 1; + } +} + class Ogg::File::FilePrivate { public: FilePrivate() : - streamSerialNumber(0), firstPageHeader(0), - lastPageHeader(0), - currentPage(0), - currentPacketPage(0) + lastPageHeader(0) { pages.setAutoDelete(true); } @@ -57,16 +66,7 @@ public: List pages; PageHeader *firstPageHeader; PageHeader *lastPageHeader; - std::vector< List > packetToPageMap; - Map dirtyPackets; - List dirtyPages; - - //! The current page for the reader -- used by nextPage() - Page *currentPage; - //! The current page for the packet parser -- used by packet() - Page *currentPacketPage; - //! The packets for the currentPacketPage -- used by packet() - ByteVectorList currentPackets; + Map dirtyPackets; }; //////////////////////////////////////////////////////////////////////////////// @@ -89,48 +89,29 @@ ByteVector Ogg::File::packet(unsigned int i) // If we haven't indexed the page where the packet we're interested in starts, // begin reading pages until we have. - while(d->packetToPageMap.size() <= i) { - if(!nextPage()) { - debug("Ogg::File::packet() -- Could not find the requested packet."); - return ByteVector(); - } + if(!readPages(i)) { + debug("Ogg::File::packet() -- Could not find the requested packet."); + return ByteVector(); } - // Start reading at the first page that contains part (or all) of this packet. - // If the last read stopped at the packet that we're interested in, don't - // reread its packet list. (This should make sequential packet reads fast.) + // Look for the first page in which the requested packet starts. - unsigned int pageIndex = d->packetToPageMap[i].front(); - if(d->currentPacketPage != d->pages[pageIndex]) { - d->currentPacketPage = d->pages[pageIndex]; - d->currentPackets = d->currentPacketPage->packets(); - } + List::ConstIterator it = d->pages.begin(); + while((*it)->containsPacket(i) == Page::DoesNotContainPacket) + ++it; - // If the packet is completely contained in the first page that it's in, then - // just return it now. - - if(d->currentPacketPage->containsPacket(i) & Page::CompletePacket) - return d->currentPackets[i - d->currentPacketPage->firstPacketIndex()]; + // If the packet is completely contained in the first page that it's in. // If the packet is *not* completely contained in the first page that it's a // part of then that packet trails off the end of the page. Continue appending // the pages' packet data until we hit a page that either does not end with the // packet that we're fetching or where the last packet is complete. - ByteVector packet = d->currentPackets.back(); - while(d->currentPacketPage->containsPacket(i) & Page::EndsWithPacket && - !d->currentPacketPage->header()->lastPacketCompleted()) - { - pageIndex++; - if(pageIndex == d->pages.size()) { - if(!nextPage()) { - debug("Ogg::File::packet() -- Could not find the requested packet."); - return ByteVector(); - } - } - d->currentPacketPage = d->pages[pageIndex]; - d->currentPackets = d->currentPacketPage->packets(); - packet.append(d->currentPackets.front()); + ByteVector packet = (*it)->packets()[i - (*it)->firstPacketIndex()]; + + while(nextPacketIndex(*it) <= i) { + ++it; + packet.append((*it)->packets().front()); } return packet; @@ -138,45 +119,37 @@ ByteVector Ogg::File::packet(unsigned int i) void Ogg::File::setPacket(unsigned int i, const ByteVector &p) { - while(d->packetToPageMap.size() <= i) { - if(!nextPage()) { - debug("Ogg::File::setPacket() -- Could not set the requested packet."); - return; - } + if(!readPages(i)) { + debug("Ogg::File::setPacket() -- Could not set the requested packet."); + return; } - List::ConstIterator it = d->packetToPageMap[i].begin(); - for(; it != d->packetToPageMap[i].end(); ++it) - d->dirtyPages.sortedInsert(*it, true); - - d->dirtyPackets.insert(i, p); + d->dirtyPackets[i] = p; } const Ogg::PageHeader *Ogg::File::firstPageHeader() { - if(d->firstPageHeader) - return d->firstPageHeader->isValid() ? d->firstPageHeader : 0; + if(!d->firstPageHeader) { + const long long firstPageHeaderOffset = find("OggS"); + if(firstPageHeaderOffset < 0) + return 0; - long long firstPageHeaderOffset = find("OggS"); + d->firstPageHeader = new PageHeader(this, firstPageHeaderOffset); + } - if(firstPageHeaderOffset < 0) - return 0; - - d->firstPageHeader = new PageHeader(this, firstPageHeaderOffset); return d->firstPageHeader->isValid() ? d->firstPageHeader : 0; } const Ogg::PageHeader *Ogg::File::lastPageHeader() { - if(d->lastPageHeader) - return d->lastPageHeader->isValid() ? d->lastPageHeader : 0; + if(!d->lastPageHeader) { + const long long lastPageHeaderOffset = rfind("OggS"); + if(lastPageHeaderOffset < 0) + return 0; - long long lastPageHeaderOffset = rfind("OggS"); + d->lastPageHeader = new PageHeader(this, lastPageHeaderOffset); + } - if(lastPageHeaderOffset < 0) - return 0; - - d->lastPageHeader = new PageHeader(this, lastPageHeaderOffset); return d->lastPageHeader->isValid() ? d->lastPageHeader : 0; } @@ -187,18 +160,10 @@ bool Ogg::File::save() return false; } - List pageGroup; + Map::ConstIterator it; + for(it = d->dirtyPackets.begin(); it != d->dirtyPackets.end(); ++it) + writePacket(it->first, it->second); - for(List::ConstIterator it = d->dirtyPages.begin(); it != d->dirtyPages.end(); ++it) { - if(!pageGroup.isEmpty() && pageGroup.back() + 1 != *it) { - writePageGroup(pageGroup); - pageGroup.clear(); - } - else - pageGroup.append(*it); - } - writePageGroup(pageGroup); - d->dirtyPages.clear(); d->dirtyPackets.clear(); return true; @@ -208,225 +173,156 @@ bool Ogg::File::save() // protected members //////////////////////////////////////////////////////////////////////////////// -Ogg::File::File(FileName file) : TagLib::File(file) +Ogg::File::File(FileName file) : + TagLib::File(file), + d(new FilePrivate()) { - d = new FilePrivate; } -Ogg::File::File(IOStream *stream) : TagLib::File(stream) +Ogg::File::File(IOStream *stream) : + TagLib::File(stream), + d(new FilePrivate()) { - d = new FilePrivate; } //////////////////////////////////////////////////////////////////////////////// // private members //////////////////////////////////////////////////////////////////////////////// -bool Ogg::File::nextPage() +bool Ogg::File::readPages(unsigned int i) { - long long nextPageOffset; - int currentPacket; + while(true) { + unsigned int packetIndex; + long long offset; - if(d->pages.isEmpty()) { - currentPacket = 0; - nextPageOffset = find("OggS"); - if(nextPageOffset < 0) + if(d->pages.isEmpty()) { + packetIndex = 0; + offset = find("OggS"); + if(offset < 0) + return false; + } + else { + const Page *page = d->pages.back(); + packetIndex = nextPacketIndex(page); + offset = page->fileOffset() + page->size(); + } + + // Enough pages have been fetched. + + if(packetIndex > i) + return true; + + // Read the next page and add it to the page list. + + Page *nextPage = new Page(this, offset); + if(!nextPage->header()->isValid()) { + delete nextPage; + return false; + } + + nextPage->setFirstPacketIndex(packetIndex); + d->pages.append(nextPage); + + if(nextPage->header()->lastPageOfStream()) return false; } - else { - if(d->currentPage->header()->lastPageOfStream()) - return false; - - if(d->currentPage->header()->lastPacketCompleted()) - currentPacket = d->currentPage->firstPacketIndex() + d->currentPage->packetCount(); - else - currentPacket = d->currentPage->firstPacketIndex() + d->currentPage->packetCount() - 1; - - nextPageOffset = d->currentPage->fileOffset() + d->currentPage->size(); - } - - // Read the next page and add it to the page list. - - d->currentPage = new Page(this, nextPageOffset); - - if(!d->currentPage->header()->isValid()) { - delete d->currentPage; - d->currentPage = 0; - return false; - } - - d->currentPage->setFirstPacketIndex(currentPacket); - - if(d->pages.isEmpty()) - d->streamSerialNumber = d->currentPage->header()->streamSerialNumber(); - - d->pages.append(d->currentPage); - - // Loop through the packets in the page that we just read appending the - // current page number to the packet to page map for each packet. - - for(unsigned int i = 0; i < d->currentPage->packetCount(); i++) { - unsigned int currentPacket = d->currentPage->firstPacketIndex() + i; - if(d->packetToPageMap.size() <= currentPacket) - d->packetToPageMap.push_back(List()); - d->packetToPageMap[currentPacket].append(static_cast(d->pages.size()) - 1); - } - - return true; } -void Ogg::File::writePageGroup(const List &thePageGroup) +void Ogg::File::writePacket(unsigned int i, const ByteVector &packet) { - if(thePageGroup.isEmpty()) + if(!readPages(i)) { + debug("Ogg::File::writePacket() -- Could not find the requested packet."); return; - - - // pages in the pageGroup and packets must be equivalent - // (originalSize and size of packets would not work together), - // therefore we sometimes have to add pages to the group - List pageGroup(thePageGroup); - while (!d->pages[pageGroup.back()]->header()->lastPacketCompleted()) { - if (d->currentPage->header()->pageSequenceNumber() == pageGroup.back()) { - if (nextPage() == false) { - debug("broken ogg file"); - return; - } - pageGroup.append(d->currentPage->header()->pageSequenceNumber()); - } else { - pageGroup.append(pageGroup.back() + 1); - } } - ByteVectorList packets; + // Look for the pages where the requested packet should belong to. - // If the first page of the group isn't dirty, append its partial content here. + List::ConstIterator it = d->pages.begin(); + while((*it)->containsPacket(i) == Page::DoesNotContainPacket) + ++it; - if(!d->dirtyPages.contains(d->pages[pageGroup.front()]->firstPacketIndex())) - packets.append(d->pages[pageGroup.front()]->packets().front()); + const Page *firstPage = *it; - int previousPacket = -1; - int originalSize = 0; + while(nextPacketIndex(*it) <= i) + ++it; - for(List::ConstIterator it = pageGroup.begin(); it != pageGroup.end(); ++it) { - unsigned int firstPacket = d->pages[*it]->firstPacketIndex(); - unsigned int lastPacket = firstPacket + d->pages[*it]->packetCount() - 1; + const Page *lastPage = *it; - List::ConstIterator last = --pageGroup.end(); + // Replace the requested packet and create new pages to replace the located pages. - for(unsigned int i = firstPacket; i <= lastPacket; i++) { + ByteVectorList packets = firstPage->packets(); + packets[i - firstPage->firstPacketIndex()] = packet; - if(it == last && i == lastPacket && !d->dirtyPages.contains(i)) - packets.append(d->pages[*it]->packets().back()); - else if(int(i) != previousPacket) { - previousPacket = i; - packets.append(packet(i)); - } - } - originalSize += d->pages[*it]->size(); + if(firstPage != lastPage && lastPage->packetCount() > 2) { + ByteVectorList lastPagePackets = lastPage->packets(); + lastPagePackets.erase(lastPagePackets.begin()); + packets.append(lastPagePackets); } - const bool continued = d->pages[pageGroup.front()]->header()->firstPacketContinued(); - const bool completed = d->pages[pageGroup.back()]->header()->lastPacketCompleted(); - // TODO: This pagination method isn't accurate for what's being done here. // This should account for real possibilities like non-aligned packets and such. - List pages = Page::paginate(packets, Page::SinglePagePerGroup, - d->streamSerialNumber, pageGroup.front(), - continued, completed); + const List pages = Page::paginate(packets, + Page::SinglePagePerGroup, + firstPage->header()->streamSerialNumber(), + firstPage->pageSequenceNumber(), + firstPage->header()->firstPacketContinued(), + lastPage->header()->lastPacketCompleted()); - List renumberedPages; + // Write the pages. - // Correct the page numbering of following pages + const long long originalOffset = firstPage->fileOffset(); + const long long originalLength = lastPage->fileOffset() + lastPage->size() - originalOffset; - if (pages.back()->header()->pageSequenceNumber() != pageGroup.back()) { + long long writtenLength = 0; - // TODO: change the internal data structure so that we don't need to hold the - // complete file in memory (is unavoidable at the moment) + for(it = pages.begin(); it != pages.end(); ++it) { + const ByteVector data = (*it)->render(); - // read the complete stream - while(!d->currentPage->header()->lastPageOfStream()) { - if(nextPage() == false) { - debug("broken ogg file"); + size_t replace; + + if(writtenLength + data.size() <= originalLength) + replace = data.size(); + else if(writtenLength <= originalLength) + replace = static_cast(originalLength - writtenLength); + else + replace = 0; + + insert(data, originalOffset + writtenLength, replace); + + writtenLength += data.size(); + } + + if(writtenLength < originalLength) + removeBlock(originalOffset + writtenLength, static_cast(originalLength - writtenLength)); + + // Renumber the following pages if the pages have been split or merged. + + const int numberOfNewPages + = pages.back()->pageSequenceNumber() - lastPage->pageSequenceNumber(); + + if(numberOfNewPages != 0) { + long long pageOffset = originalOffset + writtenLength; + + while(true) { + Page page(this, pageOffset); + if(!page.header()->isValid()) break; - } - } - // create a gap for the new pages - int numberOfNewPages = pages.back()->header()->pageSequenceNumber() - pageGroup.back(); - List::ConstIterator pageIter = d->pages.begin(); - for(int i = 0; i < pageGroup.back(); i++) { - if(pageIter != d->pages.end()) { - ++pageIter; - } - else { - debug("Ogg::File::writePageGroup() -- Page sequence is broken in original file."); + page.setPageSequenceNumber(page.pageSequenceNumber() + numberOfNewPages); + const ByteVector data = page.render(); + + seek(pageOffset + 18); + writeBlock(data.mid(18, 8)); + + if(page.header()->lastPageOfStream()) break; - } - } - ++pageIter; - for(; pageIter != d->pages.end(); ++pageIter) { - Ogg::Page *newPage = - (*pageIter)->getCopyWithNewPageSequenceNumber( - (*pageIter)->header()->pageSequenceNumber() + numberOfNewPages); - - ByteVector data; - data.append(newPage->render()); - insert(data, newPage->fileOffset(), data.size()); - - renumberedPages.append(newPage); + pageOffset += page.size(); } } - // insert the new data + // Discard all the pages to keep them up-to-date by fetching them again. - ByteVector data; - for(List::ConstIterator it = pages.begin(); it != pages.end(); ++it) - data.append((*it)->render()); - - // The insertion algorithms could also be improve to queue and prioritize data - // on the way out. Currently it requires rewriting the file for every page - // group rather than just once; however, for tagging applications there will - // generally only be one page group, so it's not worth the time for the - // optimization at the moment. - - insert(data, d->pages[pageGroup.front()]->fileOffset(), originalSize); - - // Update the page index to include the pages we just created and to delete the - // old pages. - - // First step: Pages that contain the comment data - - for(List::ConstIterator it = pages.begin(); it != pages.end(); ++it) { - const unsigned int index = (*it)->header()->pageSequenceNumber(); - if(index < d->pages.size()) { - delete d->pages[index]; - d->pages[index] = *it; - } - else if(index == d->pages.size()) { - d->pages.append(*it); - } - else { - // oops - there's a hole in the sequence - debug("Ogg::File::writePageGroup() -- Page sequence is broken."); - } - } - - // Second step: the renumbered pages - - for(List::ConstIterator it = renumberedPages.begin(); it != renumberedPages.end(); ++it) { - const unsigned int index = (*it)->header()->pageSequenceNumber(); - if(index < d->pages.size()) { - delete d->pages[index]; - d->pages[index] = *it; - } - else if(index == d->pages.size()) { - d->pages.append(*it); - } - else { - // oops - there's a hole in the sequence - debug("Ogg::File::writePageGroup() -- Page sequence is broken."); - } - } + d->pages.clear(); } diff --git a/taglib/ogg/oggfile.h b/taglib/ogg/oggfile.h index c24e0153..7d889c2f 100644 --- a/taglib/ogg/oggfile.h +++ b/taglib/ogg/oggfile.h @@ -56,7 +56,7 @@ namespace TagLib { * Returns the packet contents for the i-th packet (starting from zero) * in the Ogg bitstream. * - * \warning The requires reading at least the packet header for every page + * \warning This requires reading at least the packet header for every page * up to the requested page. */ ByteVector packet(unsigned int i); @@ -107,10 +107,15 @@ namespace TagLib { File &operator=(const File &); /*! - * Reads the next page and updates the internal "current page" pointer. + * Reads the pages from the beginning of the file until enough to compose + * the requested packet. */ - bool nextPage(); - void writePageGroup(const List &group); + bool readPages(unsigned int i); + + /*! + * Writes the requested packet to the file. + */ + void writePacket(unsigned int i, const ByteVector &packet); class FilePrivate; FilePrivate *d; diff --git a/taglib/ogg/oggpage.cpp b/taglib/ogg/oggpage.cpp index 0106b18b..2bf7fa5e 100644 --- a/taglib/ogg/oggpage.cpp +++ b/taglib/ogg/oggpage.cpp @@ -23,6 +23,8 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ +#include + #include #include @@ -38,22 +40,11 @@ public: PagePrivate(File *f = 0, long long pageOffset = -1) : file(f), fileOffset(pageOffset), - packetOffset(0), header(f, pageOffset), - firstPacketIndex(-1) - { - if(file) { - packetOffset = fileOffset + header.size(); - packetSizes = header.packetSizes(); - dataSize = header.dataSize(); - } - } + firstPacketIndex(-1) {} File *file; long long fileOffset; - long long packetOffset; - int dataSize; - List packetSizes; PageHeader header; int firstPacketIndex; ByteVectorList packets; @@ -63,9 +54,9 @@ public: // public members //////////////////////////////////////////////////////////////////////////////// -Ogg::Page::Page(Ogg::File *file, long long pageOffset) +Ogg::Page::Page(Ogg::File *file, long long pageOffset) : + d(new PagePrivate(file, pageOffset)) { - d = new PagePrivate(file, pageOffset); } Ogg::Page::~Page() @@ -83,6 +74,16 @@ const Ogg::PageHeader *Ogg::Page::header() const return &d->header; } +int Ogg::Page::pageSequenceNumber() const +{ + return d->header.pageSequenceNumber(); +} + +void Ogg::Page::setPageSequenceNumber(int sequenceNumber) +{ + d->header.setPageSequenceNumber(sequenceNumber); +} + int Ogg::Page::firstPacketIndex() const { return d->firstPacketIndex; @@ -95,7 +96,7 @@ void Ogg::Page::setFirstPacketIndex(int index) Ogg::Page::ContainsPacketFlags Ogg::Page::containsPacket(int index) const { - int lastPacketIndex = d->firstPacketIndex + packetCount() - 1; + const int lastPacketIndex = d->firstPacketIndex + packetCount() - 1; if(index < d->firstPacketIndex || index > lastPacketIndex) return DoesNotContainPacket; @@ -145,7 +146,7 @@ ByteVectorList Ogg::Page::packets() const if(d->file && d->header.isValid()) { - d->file->seek(d->packetOffset); + d->file->seek(d->fileOffset + d->header.size()); List packetSizes = d->header.packetSizes(); @@ -172,8 +173,8 @@ ByteVector Ogg::Page::render() const if(d->packets.isEmpty()) { if(d->file) { - d->file->seek(d->packetOffset); - data.append(d->file->readBlock(d->dataSize)); + d->file->seek(d->fileOffset + d->header.size()); + data.append(d->file->readBlock(d->header.dataSize())); } else debug("Ogg::Page::render() -- this page is empty!"); @@ -188,9 +189,8 @@ ByteVector Ogg::Page::render() const // the entire page with the 4 bytes reserved for the checksum zeroed and then // inserted in bytes 22-25 of the page header. - ByteVector checksum = ByteVector::fromUInt32LE(data.checksum()); - for(int i = 0; i < 4; i++) - data[i + 22] = checksum[i]; + const ByteVector checksum = ByteVector::fromUInt32LE(data.checksum()); + std::copy(checksum.begin(), checksum.end(), data.begin() + 22); return data; } @@ -203,80 +203,67 @@ List Ogg::Page::paginate(const ByteVectorList &packets, bool lastPacketCompleted, bool containsLastPacket) { + // SplitSize must be a multiple of 255 in order to get the lacing values right + // create pages of about 8KB each + + static const unsigned int SplitSize = 32 * 255; + + // Force repagination if the packets are too large for a page. + + if(strategy != Repaginate) { + + size_t totalSize = packets.size(); + for(ByteVectorList::ConstIterator it = packets.begin(); it != packets.end(); ++it) + totalSize += it->size(); + + if(totalSize > 255 * 255) + strategy = Repaginate; + } + List l; - size_t totalSize = 0; - - for(ByteVectorList::ConstIterator it = packets.begin(); it != packets.end(); ++it) - totalSize += (*it).size(); - // Handle creation of multiple pages with appropriate pagination. - if(strategy == Repaginate || totalSize + packets.size() > 255 * 255) { - // SPLITSIZE must be a multiple of 255 in order to get the lacing values right - // create pages of about 8KB each -#define SPLITSIZE (32*255) + if(strategy == Repaginate) { - int pageIndex = 0; + int pageIndex = firstPage; for(ByteVectorList::ConstIterator it = packets.begin(); it != packets.end(); ++it) { - bool continued = false; + + const bool lastPacketInList = (it == --packets.end()); // mark very first packet? - if(firstPacketContinued && it==packets.begin()) { - continued = true; - } - // append to buf - ByteVector packetBuf; - packetBuf.append(*it); + bool continued = (firstPacketContinued && it == packets.begin()); + unsigned int pos = 0; - while(packetBuf.size() > SPLITSIZE) { - // output a Page - ByteVector packetForOnePage; - packetForOnePage.resize(SPLITSIZE); - std::copy(packetBuf.begin(), packetBuf.begin() + SPLITSIZE, packetForOnePage.begin()); + while(pos < it->size()) { + + const bool lastSplit = (pos + SplitSize >= it->size()); ByteVectorList packetList; - packetList.append(packetForOnePage); - Page *p = new Page(packetList, streamSerialNumber, firstPage+pageIndex, continued, false, false); - l.append(p); + packetList.append(it->mid(pos, SplitSize)); + l.append(new Page(packetList, + streamSerialNumber, + pageIndex, + continued, + lastSplit && (lastPacketInList ? lastPacketCompleted : true), + lastSplit && (containsLastPacket && lastPacketInList))); pageIndex++; continued = true; - packetBuf = packetBuf.mid(SPLITSIZE); + + pos += SplitSize; } - - ByteVectorList::ConstIterator jt = it; - ++jt; - bool lastPacketInList = (jt == packets.end()); - - // output a page for the rest (we output one packet per page, so this one should be completed) - ByteVectorList packetList; - packetList.append(packetBuf); - - bool isVeryLastPacket = false; - if(containsLastPacket) { - // mark the very last output page as last of stream - ByteVectorList::ConstIterator jt = it; - ++jt; - if(jt == packets.end()) { - isVeryLastPacket = true; - } - } - - Page *p = new Page(packetList, streamSerialNumber, firstPage+pageIndex, continued, - lastPacketInList ? lastPacketCompleted : true, - isVeryLastPacket); - pageIndex++; - - l.append(p); } } else { - Page *p = new Page(packets, streamSerialNumber, firstPage, firstPacketContinued, - lastPacketCompleted, containsLastPacket); - l.append(p); + l.append(new Page(packets, + streamSerialNumber, + firstPage, + firstPacketContinued, + lastPacketCompleted, + containsLastPacket)); } return l; @@ -314,13 +301,9 @@ Ogg::Page::Page(const ByteVectorList &packets, int pageNumber, bool firstPacketContinued, bool lastPacketCompleted, - bool containsLastPacket) + bool containsLastPacket) : + d(new PagePrivate()) { - d = new PagePrivate; - - ByteVector data; - List packetSizes; - d->header.setFirstPageOfStream(pageNumber == 0 && !firstPacketContinued); d->header.setLastPageOfStream(containsLastPacket); d->header.setFirstPacketContinued(firstPacketContinued); @@ -330,6 +313,9 @@ Ogg::Page::Page(const ByteVectorList &packets, // Build a page from the list of packets. + ByteVector data; + List packetSizes; + for(ByteVectorList::ConstIterator it = packets.begin(); it != packets.end(); ++it) { packetSizes.append(static_cast((*it).size())); data.append(*it); @@ -337,4 +323,3 @@ Ogg::Page::Page(const ByteVectorList &packets, d->packets = packets; d->header.setPacketSizes(packetSizes); } - diff --git a/taglib/ogg/oggpage.h b/taglib/ogg/oggpage.h index 664c0cae..cc633180 100644 --- a/taglib/ogg/oggpage.h +++ b/taglib/ogg/oggpage.h @@ -70,11 +70,28 @@ namespace TagLib { */ const PageHeader *header() const; + /*! + * Returns the index of the page within the Ogg stream. This helps make it + * possible to determine if pages have been lost. + * + * \see setPageSequenceNumber() + */ + int pageSequenceNumber() const; + + /*! + * Sets the page's position in the stream to \a sequenceNumber. + * + * \see pageSequenceNumber() + */ + void setPageSequenceNumber(int sequenceNumber); + /*! * Returns a copy of the page with \a sequenceNumber set as sequence number. * * \see header() * \see PageHeader::setPageSequenceNumber() + * + * \deprecated */ Page* getCopyWithNewPageSequenceNumber(int sequenceNumber); diff --git a/taglib/ogg/oggpageheader.cpp b/taglib/ogg/oggpageheader.cpp index 0b9dd177..7fb0f0bb 100644 --- a/taglib/ogg/oggpageheader.cpp +++ b/taglib/ogg/oggpageheader.cpp @@ -23,8 +23,6 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ -#include - #include #include @@ -39,9 +37,7 @@ using namespace TagLib; class Ogg::PageHeader::PageHeaderPrivate { public: - PageHeaderPrivate(File *f, long long pageOffset) : - file(f), - fileOffset(pageOffset), + PageHeaderPrivate() : isValid(false), firstPacketContinued(false), lastPacketCompleted(false), @@ -51,11 +47,8 @@ public: streamSerialNumber(0), pageSequenceNumber(-1), size(0), - dataSize(0) - {} + dataSize(0) {} - File *file; - long long fileOffset; bool isValid; List packetSizes; bool firstPacketContinued; @@ -73,11 +66,11 @@ public: // public members //////////////////////////////////////////////////////////////////////////////// -Ogg::PageHeader::PageHeader(Ogg::File *file, long long pageOffset) +Ogg::PageHeader::PageHeader(Ogg::File *file, long long pageOffset) : + d(new PageHeaderPrivate()) { - d = new PageHeaderPrivate(file, pageOffset); if(file && pageOffset >= 0) - read(); + read(file, pageOffset); } Ogg::PageHeader::~PageHeader() @@ -184,7 +177,7 @@ ByteVector Ogg::PageHeader::render() const { ByteVector data; - // capture patern + // capture pattern data.append("OggS"); @@ -232,14 +225,14 @@ ByteVector Ogg::PageHeader::render() const // private members //////////////////////////////////////////////////////////////////////////////// -void Ogg::PageHeader::read() +void Ogg::PageHeader::read(Ogg::File *file, long long pageOffset) { - d->file->seek(d->fileOffset); + file->seek(pageOffset); // An Ogg page header is at least 27 bytes, so we'll go ahead and read that // much and then get the rest when we're ready for it. - ByteVector data = d->file->readBlock(27); + const ByteVector data = file->readBlock(27); // Sanity check -- make sure that we were in fact able to read as much data as // we asked for and that the page begins with "OggS". @@ -249,11 +242,11 @@ void Ogg::PageHeader::read() return; } - std::bitset<8> flags(data[5]); + const std::bitset<8> flags(data[5]); d->firstPacketContinued = flags.test(0); - d->firstPageOfStream = flags.test(1); - d->lastPageOfStream = flags.test(2); + d->firstPageOfStream = flags.test(1); + d->lastPageOfStream = flags.test(2); d->absoluteGranularPosition = data.toInt64LE(6); d->streamSerialNumber = data.toUInt32LE(14); @@ -265,7 +258,7 @@ void Ogg::PageHeader::read() int pageSegmentCount = static_cast(data[26]); - ByteVector pageSegments = d->file->readBlock(pageSegmentCount); + const ByteVector pageSegments = file->readBlock(pageSegmentCount); // Another sanity check. @@ -302,21 +295,17 @@ ByteVector Ogg::PageHeader::lacingValues() const { ByteVector data; - List sizes = d->packetSizes; - for(List::ConstIterator it = sizes.begin(); it != sizes.end(); ++it) { + for(List::ConstIterator it = d->packetSizes.begin(); it != d->packetSizes.end(); ++it) { // The size of a packet in an Ogg page is indicated by a series of "lacing // values" where the sum of the values is the packet size in bytes. Each of // these values is a byte. A value of less than 255 (0xff) indicates the end // of the packet. - div_t n = div(*it, 255); + data.resize(data.size() + (*it / 255), '\xff'); - for(int i = 0; i < n.quot; i++) - data.append(static_cast(255)); - - if(it != --sizes.end() || d->lastPacketCompleted) - data.append(static_cast(n.rem)); + if(it != --d->packetSizes.end() || d->lastPacketCompleted) + data.append(static_cast(*it % 255)); } return data; diff --git a/taglib/ogg/oggpageheader.h b/taglib/ogg/oggpageheader.h index 3ff4e835..c6f6a056 100644 --- a/taglib/ogg/oggpageheader.h +++ b/taglib/ogg/oggpageheader.h @@ -219,7 +219,7 @@ namespace TagLib { PageHeader(const PageHeader &); PageHeader &operator=(const PageHeader &); - void read(); + void read(Ogg::File *file, long long pageOffset); ByteVector lacingValues() const; class PageHeaderPrivate; diff --git a/tests/data/invalid-frames.mp3 b/tests/data/invalid-frames.mp3 index a6dc1117..c076712c 100644 Binary files a/tests/data/invalid-frames.mp3 and b/tests/data/invalid-frames.mp3 differ diff --git a/tests/data/invalid-frames2.mp3 b/tests/data/invalid-frames2.mp3 new file mode 100644 index 00000000..01976fc5 Binary files /dev/null and b/tests/data/invalid-frames2.mp3 differ diff --git a/tests/test_flac.cpp b/tests/test_flac.cpp index 996dc10c..e03c89f7 100644 --- a/tests/test_flac.cpp +++ b/tests/test_flac.cpp @@ -36,6 +36,7 @@ class TestFLAC : public CppUnit::TestFixture CPPUNIT_TEST(testSaveID3v1); CPPUNIT_TEST(testUpdateID3v2); CPPUNIT_TEST(testEmptyID3v2); + CPPUNIT_TEST(testStripTags); CPPUNIT_TEST_SUITE_END(); public: @@ -416,6 +417,57 @@ public: } } + void testStripTags() + { + ScopedFileCopy copy("silence-44-s", ".flac"); + + { + FLAC::File f(copy.fileName().c_str()); + f.xiphComment(true)->setTitle("XiphComment Title"); + f.ID3v1Tag(true)->setTitle("ID3v1 Title"); + f.ID3v2Tag(true)->setTitle("ID3v2 Title"); + f.save(); + } + { + FLAC::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.hasXiphComment()); + CPPUNIT_ASSERT(f.hasID3v1Tag()); + CPPUNIT_ASSERT(f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL(String("XiphComment Title"), f.xiphComment()->title()); + CPPUNIT_ASSERT_EQUAL(String("ID3v1 Title"), f.ID3v1Tag()->title()); + CPPUNIT_ASSERT_EQUAL(String("ID3v2 Title"), f.ID3v2Tag()->title()); + f.strip(FLAC::File::ID3v2); + f.save(); + } + { + FLAC::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.hasXiphComment()); + CPPUNIT_ASSERT(f.hasID3v1Tag()); + CPPUNIT_ASSERT(!f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL(String("XiphComment Title"), f.xiphComment()->title()); + CPPUNIT_ASSERT_EQUAL(String("ID3v1 Title"), f.ID3v1Tag()->title()); + f.strip(FLAC::File::ID3v1); + f.save(); + } + { + FLAC::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.hasXiphComment()); + CPPUNIT_ASSERT(!f.hasID3v1Tag()); + CPPUNIT_ASSERT(!f.hasID3v2Tag()); + CPPUNIT_ASSERT_EQUAL(String("XiphComment Title"), f.xiphComment()->title()); + f.strip(FLAC::File::XiphComment); + f.save(); + } + { + FLAC::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.hasXiphComment()); + CPPUNIT_ASSERT(!f.hasID3v1Tag()); + CPPUNIT_ASSERT(!f.hasID3v2Tag()); + CPPUNIT_ASSERT(f.xiphComment()->isEmpty()); + CPPUNIT_ASSERT_EQUAL(String("reference libFLAC 1.1.0 20030126"), f.xiphComment()->vendorID()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestFLAC); diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 41345f93..8ecf5447 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -717,11 +717,13 @@ public: CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TDTG")); CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TMOO")); CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TPRO")); +#ifdef NO_ITUNES_HACKS CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOA")); CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOT")); - CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSST")); CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSOP")); - } +#endif + CPPUNIT_ASSERT(!bar.ID3v2Tag()->frameListMap().contains("TSST")); +} } void testCompressedFrameWithBrokenLength() diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index a5003c52..5b58efcb 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -22,7 +22,8 @@ class TestMPEG : public CppUnit::TestFixture CPPUNIT_TEST(testAudioPropertiesXingHeaderVBR); CPPUNIT_TEST(testAudioPropertiesVBRIHeader); CPPUNIT_TEST(testAudioPropertiesNoVBRHeaders); - CPPUNIT_TEST(testSkipInvalidFrames); + CPPUNIT_TEST(testSkipInvalidFrames1); + CPPUNIT_TEST(testSkipInvalidFrames2); CPPUNIT_TEST(testVersion2DurationWithXingHeader); CPPUNIT_TEST(testSaveID3v24); CPPUNIT_TEST(testSaveID3v24WrongParam); @@ -93,35 +94,43 @@ public: CPPUNIT_ASSERT(!f.audioProperties()->xingHeader()); long long last = f.lastFrameOffset(); + MPEG::Header lastHeader(&f, last, false); - f.seek(last); - MPEG::Header lastHeader(f.readBlock(4)); - - while (!lastHeader.isValid()) { - + while(!lastHeader.isValid()) { last = f.previousFrameOffset(last); - - f.seek(last); - lastHeader = MPEG::Header(f.readBlock(4)); + lastHeader = MPEG::Header(&f, last, false); } CPPUNIT_ASSERT_EQUAL(28213LL, last); CPPUNIT_ASSERT_EQUAL(209, lastHeader.frameLength()); } - void testSkipInvalidFrames() + void testSkipInvalidFrames1() { MPEG::File f(TEST_FILE_PATH_C("invalid-frames.mp3")); CPPUNIT_ASSERT(f.audioProperties()); CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->length()); CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->lengthInSeconds()); - CPPUNIT_ASSERT_EQUAL(393, f.audioProperties()->lengthInMilliseconds()); + CPPUNIT_ASSERT_EQUAL(392, f.audioProperties()->lengthInMilliseconds()); CPPUNIT_ASSERT_EQUAL(160, f.audioProperties()->bitrate()); CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); CPPUNIT_ASSERT(!f.audioProperties()->xingHeader()); } + void testSkipInvalidFrames2() + { + MPEG::File f(TEST_FILE_PATH_C("invalid-frames2.mp3")); + CPPUNIT_ASSERT(f.audioProperties()); + CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->length()); + CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->lengthInSeconds()); + CPPUNIT_ASSERT_EQUAL(314, f.audioProperties()->lengthInMilliseconds()); + CPPUNIT_ASSERT_EQUAL(192, f.audioProperties()->bitrate()); + CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); + CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); + CPPUNIT_ASSERT(!f.audioProperties()->xingHeader()); + } + void testVersion2DurationWithXingHeader() { MPEG::File f(TEST_FILE_PATH_C("mpeg2.mp3"));