From 02eac18d0000b6ea9c1e04f080267e9691d08b70 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 27 Nov 2015 10:31:09 +0900 Subject: [PATCH] Fix a segfault when saving an Ogg file repeatedly. This also reduces memory usage when reading/writing Ogg files. Especially, it stops holding an entire file when renumbering Ogg pages. --- taglib/ogg/oggfile.cpp | 388 +++++++++++++---------------------- taglib/ogg/oggfile.h | 13 +- taglib/ogg/oggpage.cpp | 147 ++++++------- taglib/ogg/oggpage.h | 17 ++ taglib/ogg/oggpageheader.cpp | 45 ++-- taglib/ogg/oggpageheader.h | 2 +- 6 files changed, 252 insertions(+), 360 deletions(-) diff --git a/taglib/ogg/oggfile.cpp b/taglib/ogg/oggfile.cpp index 4b105b9a..50a1c0b2 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.) - - unsigned int pageIndex = d->packetToPageMap[i].front(); - if(d->currentPacketPage != d->pages[pageIndex]) { - d->currentPacketPage = d->pages[pageIndex]; - d->currentPackets = d->currentPacketPage->packets(); - } + // Look for the first page in which the requested packet starts. - // If the packet is completely contained in the first page that it's in, then - // just return it now. + List::ConstIterator it = d->pages.begin(); + while((*it)->containsPacket(i) == Page::DoesNotContainPacket) + ++it; - 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 firstPageHeaderOffset = find("OggS"); + if(firstPageHeaderOffset < 0) + return 0; - long firstPageHeaderOffset = find("OggS"); - - if(firstPageHeaderOffset < 0) - return 0; + d->firstPageHeader = new PageHeader(this, firstPageHeaderOffset); + } - 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 lastPageHeaderOffset = rfind("OggS"); + if(lastPageHeaderOffset < 0) + return 0; - long lastPageHeaderOffset = rfind("OggS"); - - if(lastPageHeaderOffset < 0) - return 0; + d->lastPageHeader = new PageHeader(this, lastPageHeaderOffset); + } - 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 nextPageOffset; - int currentPacket; - - if(d->pages.isEmpty()) { - currentPacket = 0; - nextPageOffset = find("OggS"); - if(nextPageOffset < 0) - 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); + while(true) { + unsigned int packetIndex; + long offset; + + 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(); + } - if(!d->currentPage->header()->isValid()) { - delete d->currentPage; - d->currentPage = 0; - return false; - } + // Enough pages have been fetched. - d->currentPage->setFirstPacketIndex(currentPacket); + if(packetIndex > i) + return true; - if(d->pages.isEmpty()) - d->streamSerialNumber = d->currentPage->header()->streamSerialNumber(); + // Read the next page and add it to the page list. - d->pages.append(d->currentPage); + Page *nextPage = new Page(this, offset); + if(!nextPage->header()->isValid()) { + delete nextPage; + return false; + } - // 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. + nextPage->setFirstPacketIndex(packetIndex); + d->pages.append(nextPage); - 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(d->pages.size() - 1); + if(nextPage->header()->lastPageOfStream()) + return false; } - - 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 unsigned long originalOffset = firstPage->fileOffset(); + const unsigned long originalLength = lastPage->fileOffset() + lastPage->size() - originalOffset; - if (pages.back()->header()->pageSequenceNumber() != pageGroup.back()) { + unsigned 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"); - break; - } - } + unsigned long replace; - // 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."); - break; - } - } - - ++pageIter; - for(; pageIter != d->pages.end(); ++pageIter) { - Ogg::Page *newPage = - (*pageIter)->getCopyWithNewPageSequenceNumber( - (*pageIter)->header()->pageSequenceNumber() + numberOfNewPages); + if(writtenLength + data.size() <= originalLength) + replace = data.size(); + else if(writtenLength <= originalLength) + replace = originalLength - writtenLength; + else + replace = 0; - ByteVector data; - data.append(newPage->render()); - insert(data, newPage->fileOffset(), data.size()); + insert(data, originalOffset + writtenLength, replace); - renumberedPages.append(newPage); - } + writtenLength += data.size(); } - // insert the new data + if(writtenLength < originalLength) + removeBlock(originalOffset + writtenLength, originalLength - writtenLength); - ByteVector data; - for(List::ConstIterator it = pages.begin(); it != pages.end(); ++it) - data.append((*it)->render()); + // Renumber the following pages if the pages have been split or merged. - // 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. + const int numberOfNewPages + = pages.back()->pageSequenceNumber() - lastPage->pageSequenceNumber(); - insert(data, d->pages[pageGroup.front()]->fileOffset(), originalSize); + if(numberOfNewPages != 0) { + long pageOffset = originalOffset + writtenLength; - // Update the page index to include the pages we just created and to delete the - // old pages. + while(true) { + Page page(this, pageOffset); + if(!page.header()->isValid()) + break; - // First step: Pages that contain the comment data + page.setPageSequenceNumber(page.pageSequenceNumber() + numberOfNewPages); + const ByteVector data = page.render(); - 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."); - } - } + seek(pageOffset + 18); + writeBlock(data.mid(18, 8)); - // Second step: the renumbered pages + if(page.header()->lastPageOfStream()) + break; - 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."); + pageOffset += page.size(); } } + + // Discard all the pages to keep them up-to-date by fetching them again. + + 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 0268c739..9f3b3dc4 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 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 fileOffset; - 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 pageOffset) +Ogg::Page::Page(Ogg::File *file, 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::fromUInt(data.checksum(), false); - for(int i = 0; i < 4; i++) - data[i + 22] = checksum[i]; + const ByteVector checksum = ByteVector::fromUInt(data.checksum(), false); + 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) { - List l; + // 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; - int totalSize = 0; + // 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; + } - for(ByteVectorList::ConstIterator it = packets.begin(); it != packets.end(); ++it) - totalSize += (*it).size(); + List l; // 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()) { - ByteVectorList packetList; - packetList.append(packetForOnePage); - Page *p = new Page(packetList, streamSerialNumber, firstPage+pageIndex, continued, false, false); - l.append(p); + const bool lastSplit = (pos + SplitSize >= it->size()); + ByteVectorList packetList; + 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); - } - 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; - } + pos += SplitSize; } - - 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((*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 d4bb34d2..3fbf75d7 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 43ed2a00..b867567c 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 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 fileOffset; bool isValid; List packetSizes; bool firstPacketContinued; @@ -73,11 +66,11 @@ public: // public members //////////////////////////////////////////////////////////////////////////////// -Ogg::PageHeader::PageHeader(Ogg::File *file, long pageOffset) +Ogg::PageHeader::PageHeader(Ogg::File *file, 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 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.toLongLong(6, false); d->streamSerialNumber = data.toUInt(14, false); @@ -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); - - for(int i = 0; i < n.quot; i++) - data.append(static_cast(255)); + data.resize(data.size() + (*it / 255), '\xff'); - 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 8ff1232b..42f67307 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 pageOffset); ByteVector lacingValues() const; class PageHeaderPrivate; -- 2.40.0