]> granicus.if.org Git - taglib/commitdiff
Fix a segfault when saving an Ogg file repeatedly.
authorTsuda Kageyu <tsuda.kageyu@gmail.com>
Fri, 27 Nov 2015 01:31:09 +0000 (10:31 +0900)
committerTsuda Kageyu <tsuda.kageyu@gmail.com>
Wed, 23 Dec 2015 23:58:56 +0000 (08:58 +0900)
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
taglib/ogg/oggfile.h
taglib/ogg/oggpage.cpp
taglib/ogg/oggpage.h
taglib/ogg/oggpageheader.cpp
taglib/ogg/oggpageheader.h

index 4b105b9adf8b67321ce3ecb8efdcc930c25d3d07..50a1c0b232f334fb87c3787bef089942825174b0 100644 (file)
 
 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<Page *> pages;
   PageHeader *firstPageHeader;
   PageHeader *lastPageHeader;
-  std::vector< List<int> > packetToPageMap;
-  Map<int, ByteVector> dirtyPackets;
-  List<int> 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<unsigned int, ByteVector> 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<Page *>::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<int>::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<int> pageGroup;
+  Map<unsigned int, ByteVector>::ConstIterator it;
+  for(it = d->dirtyPackets.begin(); it != d->dirtyPackets.end(); ++it)
+    writePacket(it->first, it->second);
 
-  for(List<int>::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<int>());
-    d->packetToPageMap[currentPacket].append(d->pages.size() - 1);
+    if(nextPage->header()->lastPageOfStream())
+      return false;
   }
-
-  return true;
 }
 
-void Ogg::File::writePageGroup(const List<int> &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<int> 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<Page *>::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<int>::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<int>::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<Page *> pages = Page::paginate(packets, Page::SinglePagePerGroup,
-                                      d->streamSerialNumber, pageGroup.front(),
-                                      continued, completed);
+  const List<Page *> pages = Page::paginate(packets,
+                                            Page::SinglePagePerGroup,
+                                            firstPage->header()->streamSerialNumber(),
+                                            firstPage->pageSequenceNumber(),
+                                            firstPage->header()->firstPacketContinued(),
+                                            lastPage->header()->lastPacketCompleted());
 
-  List<Page *> 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<Page *>::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<Page *>::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<Page *>::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<Page *>::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();
 }
index c24e015364221120a919a72e4c12e2a6995a919c..7d889c2f2b97271df17d20ca7a37d3942a1e67c7 100644 (file)
@@ -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<int> &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;
index 0268c739563785c737852130baf693be9925e7ec..9f3b3dc484f33cf0dd3211cc700ce7d53b284629 100644 (file)
@@ -23,6 +23,8 @@
  *   http://www.mozilla.org/MPL/                                           *
  ***************************************************************************/
 
+#include <algorithm>
+
 #include <tstring.h>
 #include <tdebug.h>
 
@@ -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<int> 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<int> 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 *> Ogg::Page::paginate(const ByteVectorList &packets,
                                       bool lastPacketCompleted,
                                       bool containsLastPacket)
 {
-  List<Page *> 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<Page *> 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<int> 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<int> 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);
 }
-
index d4bb34d25d04eb1ddd8f3c7900c9891642c5fcf5..3fbf75d720445f5863b131b9c80c20a585077cb4 100644 (file)
@@ -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);
 
index 43ed2a0067d793b5b8589e3da043a894a6c8e772..b867567cb6b9fbd2faac044ff6b4336fb5e35c01 100644 (file)
@@ -23,8 +23,6 @@
  *   http://www.mozilla.org/MPL/                                           *
  ***************************************************************************/
 
-#include <stdlib.h>
-
 #include <bitset>
 
 #include <tstring.h>
@@ -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<int> 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<unsigned char>(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<int> sizes = d->packetSizes;
-  for(List<int>::ConstIterator it = sizes.begin(); it != sizes.end(); ++it) {
+  for(List<int>::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<unsigned char>(255));
+    data.resize(data.size() + (*it / 255), '\xff');
 
-    if(it != --sizes.end() || d->lastPacketCompleted)
-      data.append(static_cast<unsigned char>(n.rem));
+    if(it != --d->packetSizes.end() || d->lastPacketCompleted)
+      data.append(static_cast<unsigned char>(*it % 255));
   }
 
   return data;
index 8ff1232b4f66415930bba1196ac451130d2babd4..42f673075cb5ad548b718ed0df3fd41af827f344 100644 (file)
@@ -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;