ogg_pager: Store segment table in PageHeader

Author: Serial-ATA

ogg_pager/src/header.rs

@@ -4,8 +4,10 @@ use std::io::{Read, Seek};
 
 use byteorder::{LittleEndian, ReadBytesExt};
 
+pub const PAGE_HEADER_SIZE: usize = 27;
+
 /// An OGG page header
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[derive(Clone, PartialEq, Eq, Debug)]
 pub struct PageHeader {
 	/// The position in the stream the page started at
 	pub start: u64,
@@ -16,6 +18,7 @@ pub struct PageHeader {
 	pub stream_serial: u32,
 	/// The page's sequence number
 	pub sequence_number: u32,
+	pub(crate) segments: Vec<u8>,
 	pub(crate) checksum: u32,
 }
 
@@ -34,6 +37,7 @@ impl PageHeader {
 			abgp,
 			stream_serial,
 			sequence_number,
+			segments: Vec::new(),
 			checksum: 0,
 		}
 	}
@@ -46,7 +50,7 @@ impl PageHeader {
 	/// * [`PageError::InvalidVersion`]
 	/// * [`PageError::BadSegmentCount`]
 	/// * Reader does not have enough data
-	pub fn read<R>(data: &mut R) -> Result<(Self, Vec<u8>)>
+	pub fn read<R>(data: &mut R) -> Result<Self>
 	where
 		R: Read + Seek,
 	{
@@ -88,10 +92,16 @@ impl PageHeader {
 			abgp,
 			stream_serial,
 			sequence_number,
+			segments: segment_table,
 			checksum,
 		};
 
-		Ok((header, segment_table))
+		Ok(header)
+	}
+
+	/// Returns the size of the page content, excluding the header
+	pub fn content_size(&self) -> usize {
+		self.segments.iter().map(|&b| usize::from(b)).sum::<usize>()
 	}
 
 	/// Returns the page's header type flag

ogg_pager/src/lib.rs

@@ -6,11 +6,11 @@ mod header;
 mod packets;
 mod paginate;
 
-use std::io::{Read, Seek, SeekFrom};
+use std::io::{Read, Seek};
 
 pub use crc::crc32;
 pub use error::{PageError, Result};
-pub use header::PageHeader;
+pub use header::{PageHeader, PAGE_HEADER_SIZE};
 pub use packets::{Packets, PacketsIter};
 pub use paginate::paginate;
 
@@ -27,7 +27,6 @@ pub const CONTAINS_LAST_PAGE_OF_BITSTREAM: u8 = 0x04;
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct Page {
 	content: Vec<u8>,
-	segments: Vec<u8>,
 	header: PageHeader,
 	/// The position in the stream the page ended
 	pub end: u64,
@@ -49,7 +48,10 @@ impl Page {
 	/// NOTE: This will write the checksum as is. It is likely [`Page::gen_crc`] will have
 	/// to be used prior.
 	pub fn as_bytes(&self) -> Vec<u8> {
-		let mut bytes = Vec::with_capacity(27 + self.segments.len() + self.content.len());
+		let segment_table = &self.header.segments;
+		let num_segments = segment_table.len();
+		let mut bytes =
+			Vec::with_capacity(PAGE_HEADER_SIZE + num_segments + self.header.content_size());
 
 		bytes.extend(b"OggS");
 		bytes.push(0); // Version
@@ -58,8 +60,8 @@ impl Page {
 		bytes.extend(self.header.stream_serial.to_le_bytes());
 		bytes.extend(self.header.sequence_number.to_le_bytes());
 		bytes.extend(self.header.checksum.to_le_bytes());
-		bytes.push(self.segments.len() as u8);
-		bytes.extend(&self.segments);
+		bytes.push(num_segments as u8);
+		bytes.extend(segment_table);
 		bytes.extend(self.content.iter());
 
 		bytes
@@ -73,27 +75,19 @@ impl Page {
 	///
 	/// * [`std::io::Error`]
 	/// * [`PageError`]
-	pub fn read<V>(data: &mut V, skip_content: bool) -> Result<Self>
+	pub fn read<V>(data: &mut V) -> Result<Self>
 	where
 		V: Read + Seek,
 	{
-		let (header, segments) = PageHeader::read(data)?;
+		let header = PageHeader::read(data)?;
 
-		let mut content: Vec<u8> = Vec::new();
-		let content_len: u16 = segments.iter().map(|&b| u16::from(b)).sum();
-
-		if skip_content {
-			data.seek(SeekFrom::Current(i64::from(content_len)))?;
-		} else {
-			content = vec![0; content_len as usize];
-			data.read_exact(&mut content)?;
-		}
+		let mut content = vec![0; header.content_size()];
+		data.read_exact(&mut content)?;
 
 		let end = data.stream_position()?;
 
 		Ok(Page {
 			content,
-			segments,
 			header,
 			end,
 		})
@@ -131,13 +125,13 @@ impl Page {
 
 			let mut p = Page {
 				content: content[remaining..].to_vec(),
-				segments: segment_table(remaining),
 				header: PageHeader {
 					start: self.end,
 					header_type_flag: 1,
 					abgp: 0,
 					stream_serial: self.header.stream_serial,
 					sequence_number: self.header.sequence_number + 1,
+					segments: segment_table(remaining),
 					checksum: 0,
 				},
 				end: self.header().start + content.len() as u64,
@@ -201,21 +195,21 @@ mod tests {
 				0x4F, 0x70, 0x75, 0x73, 0x48, 0x65, 0x61, 0x64, 0x01, 0x02, 0x38, 0x01, 0x80, 0xBB,
 				0, 0, 0, 0, 0,
 			],
-			segments: vec![19],
 			header: PageHeader {
 				start: 0,
 				header_type_flag: 2,
 				abgp: 0,
 				stream_serial: 1759377061,
 				sequence_number: 0,
+				segments: vec![19],
 				checksum: 3579522525,
 			},
 			end: 47,
 		};
 
 		let content = std::fs::read("test_assets/opus_ident_header.page").unwrap();
 
-		let page = Page::read(&mut Cursor::new(content), false).unwrap();
+		let page = Page::read(&mut Cursor::new(content)).unwrap();
 
 		assert_eq!(expected, page);
 	}

ogg_pager/src/packets.rs

@@ -84,8 +84,8 @@ impl Packets {
 		let mut packet_bytes_already_read = None;
 		let mut current_packet_content;
 		'outer: loop {
-			if let Ok((_, segment_table)) = PageHeader::read(data) {
-				for i in segment_table {
+			if let Ok(header) = PageHeader::read(data) {
+				for i in header.segments {
 					packet_size += i as u64;
 
 					if i < 255 {

src/ogg/mod.rs

@@ -15,9 +15,9 @@ pub(crate) mod write;
 use crate::error::Result;
 use crate::macros::decode_err;
 
-use std::io::{Read, Seek};
+use std::io::{Read, Seek, SeekFrom};
 
-use ogg_pager::Page;
+use ogg_pager::{Page, PageHeader};
 
 // Exports
 
@@ -44,11 +44,12 @@ pub(self) fn find_last_page<R>(data: &mut R) -> Result<Page>
 where
 	R: Read + Seek,
 {
-	let mut last_page = Page::read(data, true)?;
+	let mut last_page_header = PageHeader::read(data)?;
 
-	while let Ok(page) = Page::read(data, true) {
-		last_page = page
+	while let Ok(header) = PageHeader::read(data) {
+		last_page_header = header
 	}
 
-	Ok(last_page)
+	data.seek(SeekFrom::Start(last_page_header.start))?;
+	Ok(Page::read(data)?)
 }

src/ogg/read.rs

@@ -100,7 +100,7 @@ where
 
 	// TODO: Would be nice if we didn't have to read just to seek and reread immediately
 	let start = data.stream_position()?;
-	let (first_page_header, _) = PageHeader::read(data)?;
+	let first_page_header = PageHeader::read(data)?;
 
 	data.seek(SeekFrom::Start(start))?;
 

src/ogg/write.rs

@@ -99,7 +99,7 @@ where
 
 	// Read the first page header to get the stream serial number
 	let start = file.stream_position()?;
-	let (first_page_header, _) = PageHeader::read(file)?;
+	let first_page_header = PageHeader::read(file)?;
 
 	let stream_serial = first_page_header.stream_serial;
 
@@ -142,7 +142,7 @@ where
 
 	let mut pages_reader = Cursor::new(&remaining_file_content[..]);
 	let mut idx = 0;
-	while let Ok(mut page) = Page::read(&mut pages_reader, false) {
+	while let Ok(mut page) = Page::read(&mut pages_reader) {
 		let header = page.header_mut();
 		header.sequence_number = pages_written + idx;
 		page.gen_crc();