From 67bdd5b8d12ad107d80fedddbe60db492d565f5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Thu, 3 Sep 2009 17:20:29 +0000 Subject: [PATCH] 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 + taglib/ogg/oggfile.cpp | 99 +++++++++++++++++++++++++++++++++-- taglib/ogg/oggpage.cpp | 114 +++++++++++++++++++++++++++++++++++------ taglib/ogg/oggpage.h | 8 +++ tests/CMakeLists.txt | 2 + tests/Makefile.am | 4 +- tests/test_ogg.cpp | 58 +++++++++++++++++++++ 7 files changed, 266 insertions(+), 20 deletions(-) create mode 100644 tests/test_ogg.cpp diff --git a/NEWS b/NEWS index fde8b422..097b9d29 100644 --- a/NEWS +++ b/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) diff --git a/taglib/ogg/oggfile.cpp b/taglib/ogg/oggfile.cpp index ce5ac793..de9fce96 100644 --- a/taglib/ogg/oggfile.cpp +++ b/taglib/ogg/oggfile.cpp @@ -270,11 +270,28 @@ bool Ogg::File::nextPage() return true; } -void Ogg::File::writePageGroup(const List &pageGroup) +void Ogg::File::writePageGroup(const List &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 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 &pageGroup) d->streamSerialNumber, pageGroup.front(), continued, completed); + List 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::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::ConstIterator it = pages.begin(); it != pages.end(); ++it) data.append((*it)->render()); @@ -328,9 +391,37 @@ void Ogg::File::writePageGroup(const List &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::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(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 int index = (*it)->header()->pageSequenceNumber(); + if(index < static_cast(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."); + } } } diff --git a/taglib/ogg/oggpage.cpp b/taglib/ogg/oggpage.cpp index 0b828284..9b50fef8 100644 --- a/taglib/ogg/oggpage.cpp +++ b/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::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); } + diff --git a/taglib/ogg/oggpage.h b/taglib/ogg/oggpage.h index fbe0ebc6..002d6ba6 100644 --- a/taglib/ogg/oggpage.h +++ b/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. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 41b1129f..2db60512 100644 --- a/tests/CMakeLists.txt +++ b/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) diff --git a/tests/Makefile.am b/tests/Makefile.am index 2e5197ca..f51e0765 100644 --- a/tests/Makefile.am +++ b/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 diff --git a/tests/test_ogg.cpp b/tests/test_ogg.cpp new file mode 100644 index 00000000..be81877e --- /dev/null +++ b/tests/test_ogg.cpp @@ -0,0 +1,58 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#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);