diff --git a/taglib/ogg/oggfile.cpp b/taglib/ogg/oggfile.cpp index c08ffde8..47db66c8 100644 --- a/taglib/ogg/oggfile.cpp +++ b/taglib/ogg/oggfile.cpp @@ -57,6 +57,16 @@ public: std::unique_ptr firstPageHeader; std::unique_ptr lastPageHeader; Map dirtyPackets; + + // File offset of the next page to read while scanning the file. This tracks + // the physical position independently of the logical stream's pages so that + // pages of other multiplexed streams can be skipped. + offset_t currentPageOffset { -1 }; + + // Serial number of the logical bitstream packets are read from. In a + // multiplexed Ogg stream only pages of this stream are considered. + unsigned int streamSerialNumber { 0 }; + bool streamSerialNumberSet { false }; }; //////////////////////////////////////////////////////////////////////////////// @@ -171,6 +181,38 @@ Ogg::File::File(IOStream *stream) : { } +bool Ogg::File::selectStream(const ByteVector &magic) +{ + // All beginning-of-stream pages of a (possibly multiplexed) Ogg stream + // appear at the very start of the file, before any secondary pages. Inspect + // each one's first packet and lock onto the first logical bitstream whose + // identification header matches magic. + + offset_t offset = find("OggS"); + if(offset < 0) + return false; + + while(true) { + Page page(this, offset); + if(!page.header()->isValid()) + return false; + + // Once the beginning-of-stream pages are exhausted there are no more + // logical bitstreams to discover. + if(!page.header()->firstPageOfStream()) + return false; + + const ByteVectorList packets = page.packets(); + if(!packets.isEmpty() && packets.front().startsWith(magic)) { + d->streamSerialNumber = page.header()->streamSerialNumber(); + d->streamSerialNumberSet = true; + return true; + } + + offset += page.size(); + } +} + //////////////////////////////////////////////////////////////////////////////// // private members //////////////////////////////////////////////////////////////////////////////// @@ -178,37 +220,53 @@ Ogg::File::File(IOStream *stream) : bool Ogg::File::readPages(unsigned int i) { while(true) { - unsigned int packetIndex; - offset_t offset; - if(d->pages.isEmpty()) { - packetIndex = 0; - offset = find("OggS"); - if(offset < 0) - return false; - } - else { + // If we've already indexed the page containing packet i, we're done. + + if(!d->pages.isEmpty()) { const Page *page = d->pages.back(); - packetIndex = nextPacketIndex(page); - offset = page->fileOffset() + page->size(); - - // Enough pages have been fetched. - if(packetIndex > i) { + if(nextPacketIndex(page) > i) return true; - } - else if(page->header()->lastPageOfStream()) { + if(page->header()->lastPageOfStream()) + return false; + } + + // Locate the first page in the file if we haven't started scanning yet. + + if(d->currentPageOffset < 0) { + d->currentPageOffset = find("OggS"); + if(d->currentPageOffset < 0) + return false; + } + + // Read pages until we find the next one belonging to our logical bitstream, + // skipping pages of other streams in a multiplexed Ogg stream. + + Page *nextPage; + while(true) { + nextPage = new Page(this, d->currentPageOffset); + if(!nextPage->header()->isValid()) { + delete nextPage; return false; } - } - // Read the next page and add it to the page list. + d->currentPageOffset += nextPage->size(); + + const unsigned int serial = nextPage->header()->streamSerialNumber(); + if(!d->streamSerialNumberSet) { + d->streamSerialNumber = serial; + d->streamSerialNumberSet = true; + } + + if(serial == d->streamSerialNumber) + break; - auto nextPage = new Page(this, offset); - if(!nextPage->header()->isValid()) { delete nextPage; - return false; } + const unsigned int packetIndex + = d->pages.isEmpty() ? 0 : nextPacketIndex(d->pages.back()); + nextPage->setFirstPacketIndex(packetIndex); d->pages.append(nextPage); } @@ -295,4 +353,5 @@ void Ogg::File::writePacket(unsigned int i, const ByteVector &packet) // Discard all the pages to keep them up-to-date by fetching them again. d->pages.clear(); + d->currentPageOffset = -1; } diff --git a/taglib/ogg/oggfile.h b/taglib/ogg/oggfile.h index a4d75f31..3a4849fa 100644 --- a/taglib/ogg/oggfile.h +++ b/taglib/ogg/oggfile.h @@ -105,6 +105,20 @@ namespace TagLib { */ File(IOStream *stream); + /*! + * Restricts packet parsing to the first logical bitstream whose first + * packet begins with \a magic. This is needed for multiplexed Ogg + * streams, such as an Ogg Vorbis stream muxed with an Ogg Theora video + * stream carrying cover art, where packets of the individual logical + * bitstreams are interleaved. Must be called before any packet is + * requested. + * + * Returns \c true if a matching logical bitstream was found and selected. + * If no match is found, the file falls back to the first logical + * bitstream in the file. + */ + bool selectStream(const ByteVector &magic); + private: /*! * Reads the pages from the beginning of the file until enough to compose diff --git a/taglib/ogg/vorbis/vorbisfile.cpp b/taglib/ogg/vorbis/vorbisfile.cpp index 7b5823b4..40a58b4e 100644 --- a/taglib/ogg/vorbis/vorbisfile.cpp +++ b/taglib/ogg/vorbis/vorbisfile.cpp @@ -41,8 +41,10 @@ public: namespace TagLib { /*! * Vorbis headers can be found with one type ID byte and the string "vorbis" in - * an Ogg stream. 0x03 indicates the comment header. + * an Ogg stream. 0x01 indicates the identification header and 0x03 indicates + * the comment header. */ + static constexpr char vorbisIdentificationHeaderID[] = { 0x01, 'v', 'o', 'r', 'b', 'i', 's', 0 }; static constexpr char vorbisCommentHeaderID[] = { 0x03, 'v', 'o', 'r', 'b', 'i', 's', 0 }; } // namespace TagLib @@ -119,6 +121,10 @@ bool Vorbis::File::save() void Vorbis::File::read(bool readProperties) { + // Select the Vorbis logical bitstream in case the file is a multiplexed Ogg + // stream (e.g. Vorbis audio muxed with a Theora video stream for cover art). + selectStream(vorbisIdentificationHeaderID); + ByteVector commentHeaderData = packet(1); if(commentHeaderData.mid(0, 7) != vorbisCommentHeaderID) { diff --git a/tests/data/multiplex.ogg b/tests/data/multiplex.ogg new file mode 100644 index 00000000..892bec9a Binary files /dev/null and b/tests/data/multiplex.ogg differ diff --git a/tests/test_ogg.cpp b/tests/test_ogg.cpp index 36e8e231..a55ec70b 100644 --- a/tests/test_ogg.cpp +++ b/tests/test_ogg.cpp @@ -32,6 +32,7 @@ #include "oggfile.h" #include "vorbisfile.h" #include "oggpageheader.h" +#include "plainfile.h" #include #include "utils.h" @@ -47,6 +48,7 @@ class TestOGG : public CppUnit::TestFixture CPPUNIT_TEST(testDictInterface1); CPPUNIT_TEST(testDictInterface2); CPPUNIT_TEST(testAudioProperties); + CPPUNIT_TEST(testMultiplexed); CPPUNIT_TEST(testPageChecksum); CPPUNIT_TEST(testPageGranulePosition); CPPUNIT_TEST_SUITE_END(); @@ -200,6 +202,45 @@ public: CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrateMinimum()); } + void testMultiplexed() + { + // A multiplexed Ogg stream where the Vorbis logical bitstream is not the + // first one (it is preceded by a Theora video stream). The Vorbis comment + // and audio properties must still be read from the Vorbis stream. + ScopedFileCopy copy("multiplex", ".ogg"); + string filename = copy.fileName(); + { + Vorbis::File f(filename.c_str()); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT(f.tag()); + CPPUNIT_ASSERT_EQUAL(String("Paper Lights"), f.tag()->title()); + CPPUNIT_ASSERT(f.audioProperties()); + CPPUNIT_ASSERT_EQUAL(2, f.audioProperties()->channels()); + CPPUNIT_ASSERT_EQUAL(48000, f.audioProperties()->sampleRate()); + + f.tag()->setTitle("Changed Title"); + CPPUNIT_ASSERT(f.save()); + } + { + Vorbis::File f(filename.c_str()); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT_EQUAL(String("Changed Title"), f.tag()->title()); + + f.tag()->setTitle("Paper Lights"); + CPPUNIT_ASSERT(f.save()); + } + { + Vorbis::File f(filename.c_str()); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT_EQUAL(String("Paper Lights"), f.tag()->title()); + } + + // Check if the modified file is byte for byte equal to the original file + const ByteVector origData = PlainFile(TEST_FILE_PATH_C("multiplex.ogg")).readAll(); + const ByteVector fileData = PlainFile(filename.c_str()).readAll(); + CPPUNIT_ASSERT(origData == fileData); + } + void testPageChecksum() { ScopedFileCopy copy("empty", ".ogg");