Fixed: Corrupting OPUS files on write

Author: ta264

src/TaglibSharp.Tests/FileFormats/OpusFormatTest.cs

@@ -67,5 +67,28 @@ public void TestCorruptionResistance ()
 		{
 			StandardTests.TestCorruptionResistance (TestPath.Samples + "corrupt/a.opus");
 		}
+
+		[Test]
+		public void CheckInvariantStartPosition()
+		{
+			// There was a corruption bug in OPUS file writer, the root cause of which was
+			// the Ogg Bitstream class requiring the first page of the media data to be read
+			// in order to process the tags.  Then on write this media page is incorrectly
+			// replaced with a page with absoluteGranularPosition = 0, which is not allowed.
+			//
+			// The sample file has the media beginning in the third page.  To test the fix
+			// ensure that the InvariantStartPosition is the location of the third page.
+			// Previously we read/wrote the third page and corrupted it, and InvariantStartPosition
+			// was set to the start of the fourth page.
+			//
+			// In principle the comments packet can span multiple pages, so if the test file
+			// is updated in future this may need adjusting.
+
+			var p1 = file.Find("OggS", 0);
+			var p2 = file.Find("OggS", p1 + 1);
+			var p3 = file.Find("OggS", p2 + 1);
+
+			Assert.AreEqual(p3, file.InvariantStartPosition);
+		}
 	}
 }

src/TaglibSharp/Ogg/Bitstream.cs

@@ -119,14 +119,6 @@ public bool ReadPage (Page page)
 			ByteVector[] packets = page.Packets;
 
 			for (int i = 0; i < packets.Length; i++) {
-				if ((page.Header.Flags &
-					PageFlags.FirstPacketContinued) == 0 &&
-					previous_packet != null) {
-					if (ReadPacket (previous_packet))
-						return true;
-					previous_packet = null;
-				}
-
 
 				ByteVector packet = packets[i];
 
@@ -142,9 +134,9 @@ public bool ReadPage (Page page)
 
 				previous_packet = null;
 
-				if (i == packets.Length - 1) {
+				if (i == packets.Length - 1 && !page.Header.LastPacketComplete) {
 					// If we're at the last packet of the
-					// page, store it.
+					// page and it's continued on the next page, store it.
 					previous_packet = new ByteVector (packet);
 				} else if (ReadPacket (packet)) {
 					// Otherwise, we need to process it.

src/TaglibSharp/Ogg/PageHeader.cs

@@ -115,6 +115,7 @@ public PageHeader (uint streamSerialNumber, uint pageNumber, PageFlags flags)
 			Size = 0;
 			DataSize = 0;
 			packet_sizes = new List<int> ();
+			LastPacketComplete = false;
 
 			if (pageNumber == 0 && (flags & PageFlags.FirstPacketContinued) == 0)
 				Flags |= PageFlags.FirstPageOfStream;
@@ -200,6 +201,8 @@ public PageHeader (File file, long position)
 
 			if (packet_size > 0)
 				packet_sizes.Add (packet_size);
+
+			LastPacketComplete = page_segments[page_segment_count - 1] < 255;
 		}
 
 		/// <summary>
@@ -230,6 +233,7 @@ public PageHeader (PageHeader original, uint offset, PageFlags flags)
 			Size = original.Size;
 			DataSize = original.DataSize;
 			packet_sizes = new List<int> ();
+			LastPacketComplete = false;
 
 			if (PageSequenceNumber == 0 && (flags & PageFlags.FirstPacketContinued) == 0)
 				Flags |= PageFlags.FirstPageOfStream;
@@ -257,12 +261,20 @@ public int[] PacketSizes {
 		}
 
 		/// <summary>
-		///    Gets the flags for the page described by the current
-		///    instance.
+		///	   Indicates whether the final packet is continued on the next page
 		/// </summary>
 		/// <value>
-		///    A <see cref="PageFlags" /> value containing the page
-		///    flags.
+		///	   true if the final packet is complete and not continued on the next page
+		/// </value>
+		public bool LastPacketComplete { get; private set; }
+
+		/// <summary>
+		///	   Gets the flags for the page described by the current
+		///	   instance.
+		/// </summary>
+		/// <value>
+		///	   A <see cref="PageFlags" /> value containing the page
+		///	   flags.
 		/// </value>
 		public PageFlags Flags { get; private set; }