Skip to content

Commit 53df9b4

Browse files
committed
ogg_pager: Make operations less fallible
1 parent a201ab3 commit 53df9b4

File tree

3 files changed

+21
-55
lines changed

3 files changed

+21
-55
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ lofty_attr = { path = "lofty_attr" }
2424
# Debug logging
2525
log = "0.4.17"
2626
# OGG Vorbis/Opus
27-
ogg_pager = "0.4.0"
27+
ogg_pager = { path = "ogg_pager" }
2828
# Key maps
2929
once_cell = "1.16.0"
3030
paste = "1.0.11"

ogg_pager/src/lib.rs

Lines changed: 18 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,7 @@ impl Page {
4848
///
4949
/// NOTE: This will write the checksum as is. It is likely [`Page::gen_crc`] will have
5050
/// to be used prior.
51-
///
52-
/// # Errors
53-
///
54-
/// See [`segment_table`]
55-
pub fn as_bytes(&self) -> Result<Vec<u8>> {
51+
pub fn as_bytes(&self) -> Vec<u8> {
5652
let mut bytes = Vec::with_capacity(27 + self.segments.len() + self.content.len());
5753

5854
bytes.extend(b"OggS");
@@ -66,7 +62,7 @@ impl Page {
6662
bytes.extend(&self.segments);
6763
bytes.extend(self.content.iter());
6864

69-
Ok(bytes)
65+
bytes
7066
}
7167

7268
/// Attempts to get a Page from a reader
@@ -104,37 +100,25 @@ impl Page {
104100
}
105101

106102
/// Generates the CRC checksum of the page
107-
///
108-
/// # Errors
109-
///
110-
/// See [`Page::as_bytes`]
111-
pub fn gen_crc(&mut self) -> Result<()> {
103+
pub fn gen_crc(&mut self) {
112104
// The value is computed over the entire header (with the CRC field in the header set to zero) and then continued over the page
113105
self.header.checksum = 0;
114-
self.header.checksum = crc::crc32(&self.as_bytes()?);
115-
116-
Ok(())
106+
self.header.checksum = crc::crc32(&self.as_bytes());
117107
}
118108

119109
/// Extends the Page's content, returning another Page if too much data was provided
120110
///
121111
/// This will do nothing if `content` is greater than the max page size. In this case,
122112
/// [`paginate()`] should be used.
123-
///
124-
/// # Errors
125-
///
126-
/// *Only applicable if a new page is created*:
127-
///
128-
/// See [`Page::gen_crc`]
129-
pub fn extend(&mut self, content: &[u8]) -> Result<Option<Page>> {
113+
pub fn extend(&mut self, content: &[u8]) -> Option<Page> {
130114
let self_len = self.content.len();
131115
let content_len = content.len();
132116

133117
if self_len + content_len <= MAX_CONTENT_SIZE {
134118
self.content.extend(content.iter());
135119
self.end += content_len as u64;
136120

137-
return Ok(None);
121+
return None;
138122
}
139123

140124
if content_len <= MAX_CONTENT_SIZE {
@@ -147,7 +131,7 @@ impl Page {
147131

148132
let mut p = Page {
149133
content: content[remaining..].to_vec(),
150-
segments: segment_table(remaining)?,
134+
segments: segment_table(remaining),
151135
header: PageHeader {
152136
start: self.end,
153137
header_type_flag: 1,
@@ -159,12 +143,12 @@ impl Page {
159143
end: self.header().start + content.len() as u64,
160144
};
161145

162-
p.gen_crc()?;
146+
p.gen_crc();
163147

164-
return Ok(Some(p));
148+
return Some(p);
165149
}
166150

167-
Ok(None)
151+
None
168152
}
169153

170154
/// Returns the page's content
@@ -178,34 +162,19 @@ impl Page {
178162
}
179163

180164
/// Returns the page's segment table
181-
///
182-
/// # Errors
183-
///
184-
/// See [`segment_table`]
185-
pub fn segment_table(&self) -> Result<Vec<u8>> {
165+
pub fn segment_table(&self) -> Vec<u8> {
186166
segment_table(self.content.len())
187167
}
188168
}
189169

190170
/// Creates a segment table based on the length
191-
///
192-
/// # Errors
193-
///
194-
/// `length` > [`MAX_CONTENT_SIZE`]
195-
pub fn segment_table(length: usize) -> Result<Vec<u8>> {
196-
match length {
197-
0 => return Ok(vec![1, 0]),
198-
l if l > MAX_CONTENT_SIZE => return Err(PageError::TooMuchData),
199-
_ => {},
200-
};
201-
202-
let mut last_len = (length % 255) as u8;
203-
if last_len == 0 {
204-
last_len = 255;
171+
pub fn segment_table(length: usize) -> Vec<u8> {
172+
if length == 0 {
173+
return vec![1, 0];
205174
}
206175

207-
let mut needed = (length / 255) + 1;
208-
needed = std::cmp::min(needed, 255);
176+
let last_len = (length % 255) as u8;
177+
let needed = (length / 255) + 1;
209178

210179
let mut segments = Vec::with_capacity(needed);
211180

@@ -217,7 +186,7 @@ pub fn segment_table(length: usize) -> Result<Vec<u8>> {
217186
}
218187
}
219188

220-
Ok(segments)
189+
segments
221190
}
222191

223192
#[cfg(test)]
@@ -264,10 +233,7 @@ mod tests {
264233

265234
assert_eq!(
266235
last_page_content.len() % 255,
267-
*segment_table(last_page_content.len())
268-
.unwrap()
269-
.last()
270-
.unwrap() as usize
236+
*segment_table(last_page_content.len()).last().unwrap() as usize
271237
);
272238

273239
for (i, page) in pages.into_iter().enumerate() {

ogg_pager/src/packets.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,8 @@ impl Packets {
334334
let paginated = self.paginate(stream_serial, abgp, flags)?;
335335

336336
for mut page in paginated.into_iter() {
337-
page.gen_crc()?;
338-
writer.write_all(&page.as_bytes()?)?;
337+
page.gen_crc();
338+
writer.write_all(&page.as_bytes())?;
339339
}
340340

341341
Ok(())

0 commit comments

Comments
 (0)