Split Ogg packets larger than 64kb into multiple pages

The implementation is not very efficient, but the current Ogg
code makes it hard to write it properly. :(

Patch by Marc Halbruegge
BUG:171957


git-svn-id: svn://anonsvn.kde.org/home/kde/trunk/kdesupport/taglib@1019459 283d02a7-25f6-0310-bc7c-ecb5cbfe19da

NEWS

@@ -1,6 +1,7 @@
 TagLib 1.6
 ==========
 
+ * Split Ogg packets larger than 64k into multiple pages. (BUG:171957)
  * TagLib can now use FLAC padding block. (BUG:107659)
  * ID3v2.2 frames are now not incorrectly saved. (BUG:176373)
  * Support for ID3v2.2 PIC frames. (BUG:167786)

taglib/ogg/oggfile.cpp

@@ -270,11 +270,28 @@ bool Ogg::File::nextPage()
   return true;
 }
 
-void Ogg::File::writePageGroup(const List<int> &pageGroup)
+void Ogg::File::writePageGroup(const List<int> &thePageGroup)
 {
-  if(pageGroup.isEmpty())
+  if(thePageGroup.isEmpty())
     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;
 
   // If the first page of the group isn't dirty, append its partial content here.
@@ -313,6 +330,52 @@ void Ogg::File::writePageGroup(const List<int> &pageGroup)
                                       d->streamSerialNumber, pageGroup.front(),
                                       continued, completed);
 
+  List<Page *> renumberedPages;
+
+  // Correct the page numbering of following pages
+
+  if (pages.back()->header()->pageSequenceNumber() != pageGroup.back()) {
+
+    // TODO: change the internal data structure so that we don't need to hold the 
+    // complete file in memory (is unavoidable at the moment)
+
+    // read the complete stream
+    while(!d->currentPage->header()->lastPageOfStream()) {
+      if(nextPage() == false) {
+        debug("broken ogg file");
+        break;
+      }
+    }
+
+    // create a gap for the new pages
+    int numberOfNewPages = pages.back()->header()->pageSequenceNumber() - pageGroup.back();
+    List<Page *>::Iterator 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);
+
+      ByteVector data;
+      data.append(newPage->render());
+      insert(data, newPage->fileOffset(), data.size());
+
+      renumberedPages.append(newPage);
+    }
+  }
+
+  // insert the new data
+
   ByteVector data;
   for(List<Page *>::ConstIterator it = pages.begin(); it != pages.end(); ++it)
     data.append((*it)->render());
@@ -328,9 +391,37 @@ void Ogg::File::writePageGroup(const List<int> &pageGroup)
   // 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<Page *>::ConstIterator it = pages.begin(); it != pages.end(); ++it) {
     const int index = (*it)->header()->pageSequenceNumber();
-    delete d->pages[index];
-    d->pages[index] = *it;
+    if(index < static_cast<int>(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<Page *>::ConstIterator it = renumberedPages.begin(); it != renumberedPages.end(); ++it) {
+    const int index = (*it)->header()->pageSequenceNumber();
+    if(index < static_cast<int>(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.");
+    }
   }
 }

taglib/ogg/oggpage.cpp

@@ -116,12 +116,14 @@ Ogg::Page::ContainsPacketFlags Ogg::Page::containsPacket(int index) const
     flags = ContainsPacketFlags(flags | CompletePacket);
   }
 
-  // Or if the page is (a) the first page and it's complete or (b) the last page
-  // and it's complete or (c) a page in the middle.
-
-  else if((flags & BeginsWithPacket && !d->header.firstPacketContinued()) ||
-          (flags & EndsWithPacket && d->header.lastPacketCompleted()) ||
-          (!(flags & BeginsWithPacket) && !(flags & EndsWithPacket)))
+  // Or if there is more than one page and the page is 
+  // (a) the first page and it's complete or 
+  // (b) the last page and it's complete or 
+  // (c) a page in the middle.
+  else if(packetCount() > 1 &&
+          ((flags & BeginsWithPacket && !d->header.firstPacketContinued()) ||
+           (flags & EndsWithPacket && d->header.lastPacketCompleted()) ||
+           (!(flags & BeginsWithPacket) && !(flags & EndsWithPacket))))
   {
     flags = ContainsPacketFlags(flags | CompletePacket);
   }
@@ -208,20 +210,101 @@ List<Ogg::Page *> Ogg::Page::paginate(const ByteVectorList &packets,
   for(ByteVectorList::ConstIterator it = packets.begin(); it != packets.end(); ++it)
     totalSize += (*it).size();
 
-  if(strategy == Repaginate || totalSize + packets.size() > 255 * 256) {
-    debug("Ogg::Page::paginate() -- Sorry!  Repagination is not yet implemented.");
-    return 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)
+
+    int pageIndex = 0;
+
+    for(ByteVectorList::ConstIterator it = packets.begin(); it != packets.end(); ++it) {
+      bool continued = false;
+
+      // mark very first packet?
+      if(firstPacketContinued && it==packets.begin()) {
+        continued = true;
+      }
+
+      // append to buf
+      ByteVector packetBuf;
+      packetBuf.append(*it);
+
+      while(packetBuf.size() > SPLITSIZE) {
+        // output a Page
+        ByteVector packetForOnePage;
+        packetForOnePage.resize(SPLITSIZE);
+        std::copy(packetBuf.begin(), packetBuf.begin() + SPLITSIZE, packetForOnePage.begin());
+
+        ByteVectorList packetList;
+        packetList.append(packetForOnePage);
+        Page *p = new Page(packetList, streamSerialNumber, firstPage+pageIndex, continued, false, false);
+        l.append(p);
+
+        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;
+        }
+      }
+
+      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);
   }
 
-  // TODO: Handle creation of multiple pages here with appropriate pagination.
-
-  Page *p = new Page(packets, streamSerialNumber, firstPage, firstPacketContinued,
-                     lastPacketCompleted, containsLastPacket);
-  l.append(p);
-
   return l;
 }
 
+Ogg::Page* Ogg::Page::getCopyWithNewPageSequenceNumber(int sequenceNumber)
+{
+  Page *pResultPage = NULL;
+
+  // TODO: a copy constructor would be helpful
+
+  if(d->file == 0) {
+    pResultPage = new Page(
+        d->packets,
+        d->header.streamSerialNumber(),
+        sequenceNumber,
+        d->header.firstPacketContinued(),
+        d->header.lastPacketCompleted(),
+        d->header.lastPageOfStream());
+  }
+  else
+  {
+    pResultPage = new Page(d->file, d->fileOffset);
+    pResultPage->d->header.setPageSequenceNumber(sequenceNumber);
+  }
+  return pResultPage;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // protected members
 ////////////////////////////////////////////////////////////////////////////////
@@ -254,3 +337,4 @@ Ogg::Page::Page(const ByteVectorList &packets,
   d->packets = packets;
   d->header.setPacketSizes(packetSizes);
 }
+

taglib/ogg/oggpage.h

@@ -70,6 +70,14 @@ namespace TagLib {
        */
       const PageHeader *header() const;
 
+      /*! 
+       * Returns a copy of the page with \a sequenceNumber set as sequence number.
+       * 
+       * \see header()
+       * \see PageHeader::setPageSequenceNumber()
+       */
+      Page* getCopyWithNewPageSequenceNumber(int sequenceNumber);
+
       /*!
        * Returns the index of the first packet wholly or partially contained in
        * this page.

tests/CMakeLists.txt

@@ -13,6 +13,7 @@ INCLUDE_DIRECTORIES(
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/riff/aiff
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/trueaudio
   ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/ogg
+  ${CMAKE_CURRENT_SOURCE_DIR}/../taglib/ogg/vorbis
 )
 
 SET(test_runner_SRCS
@@ -31,6 +32,7 @@ SET(test_runner_SRCS
   test_xiphcomment.cpp
   test_aiff.cpp
   test_riff.cpp
+  test_ogg.cpp
 )
 IF(WITH_MP4)
    SET(test_runner_SRCS ${test_runner_SRCS} test_mp4.cpp)

tests/Makefile.am

@@ -6,6 +6,7 @@ INCLUDES = \
         -I$(top_srcdir)/taglib/mpeg/id3v1 \
         -I$(top_srcdir)/taglib/mpeg/id3v2 \
         -I$(top_srcdir)/taglib/ogg \
+        -I$(top_srcdir)/taglib/ogg/vorbis \
         -I$(top_srcdir)/taglib/riff \
         -I$(top_srcdir)/taglib/riff/aiff \
         -I$(top_srcdir)/taglib/mpeg/id3v2/frames
@@ -24,7 +25,8 @@ test_runner_SOURCES = \
 	test_id3v2.cpp \
 	test_xiphcomment.cpp \
 	test_riff.cpp \
-	test_aiff.cpp
+	test_aiff.cpp \
+	test_ogg.cpp
 
 if build_tests
 TESTS = test_runner

tests/test_ogg.cpp

@@ -0,0 +1,58 @@
+#include <cppunit/extensions/HelperMacros.h>
+#include <string>
+#include <stdio.h>
+#include <tag.h>
+#include <tstringlist.h>
+#include <tbytevectorlist.h>
+#include <oggfile.h>
+#include <vorbisfile.h>
+#include <oggpageheader.h>
+#include "utils.h"
+
+using namespace std;
+using namespace TagLib;
+
+class TestOGG : public CppUnit::TestFixture
+{
+  CPPUNIT_TEST_SUITE(TestOGG);
+  CPPUNIT_TEST(testSimple);
+  CPPUNIT_TEST(testSplitPackets);
+  CPPUNIT_TEST_SUITE_END();
+
+public:
+
+  void testSimple()
+  {
+    string newname = copyFile("empty", ".ogg");
+
+    Vorbis::File *f = new Vorbis::File(newname.c_str());
+    f->tag()->setArtist("The Artist");
+    f->save();
+    delete f;
+
+    f = new Vorbis::File(newname.c_str());
+    CPPUNIT_ASSERT_EQUAL(String("The Artist"), f->tag()->artist());
+    delete f;
+
+    deleteFile(newname);
+  }
+
+  void testSplitPackets()
+  {
+    string newname = copyFile("empty", ".ogg");
+
+    Vorbis::File *f = new Vorbis::File(newname.c_str());
+    f->tag()->addField("test", ByteVector(128 * 1024, 'x') + ByteVector(1, '\0'));
+    f->save();
+    delete f;
+
+    f = new Vorbis::File(newname.c_str());
+    CPPUNIT_ASSERT_EQUAL(19, f->lastPageHeader()->pageSequenceNumber());
+    delete f;
+
+    deleteFile(newname);
+  }
+
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(TestOGG);