Skip to content

Commit fc3a0aa

Browse files
authored
Fix writer to skip packets with an empty payload (webrtc-rs#635)
* Fix writer to skip packets with an empty payload * Fix writer tests
1 parent 70fc5b4 commit fc3a0aa

File tree

4 files changed

+16
-24
lines changed

4 files changed

+16
-24
lines changed

media/src/io/ivf_writer/ivf_writer_test.rs

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::io::Cursor;
22

33
use super::*;
4-
use crate::error::Error;
54

65
#[test]
76
fn test_ivf_writer_add_packet_and_close() -> Result<()> {
@@ -117,26 +116,23 @@ fn test_ivf_writer_add_packet_and_close() -> Result<()> {
117116

118117
let add_packet_test_case = vec![
119118
(
120-
"IVFWriter shouldn't be able to write something an empty packet",
119+
"IVFWriter should be able to skip something an empty packet",
121120
"IVFWriter should be able to close the file",
122121
rtp::packet::Packet::default(),
123-
Some(Error::ErrInvalidNilPacket),
124122
false,
125-
0,
123+
1,
126124
),
127125
(
128126
"IVFWriter should be able to write an IVF packet",
129127
"IVFWriter should be able to close the file",
130128
valid_packet.clone(),
131-
None,
132129
false,
133130
1,
134131
),
135132
(
136133
"IVFWriter should be able to write a Keframe IVF packet",
137134
"IVFWriter should be able to close the file",
138135
keyframe_packet,
139-
None,
140136
true,
141137
2,
142138
),
@@ -155,20 +151,16 @@ fn test_ivf_writer_add_packet_and_close() -> Result<()> {
155151
unused: 0, // Unused
156152
};
157153

158-
for (msg1, _msg2, packet, err, seen_key_frame, count) in add_packet_test_case {
154+
for (msg1, _msg2, packet, seen_key_frame, count) in add_packet_test_case {
159155
let mut writer = IVFWriter::new(Cursor::new(Vec::<u8>::new()), &header)?;
160156
assert!(
161157
!writer.seen_key_frame,
162158
"Writer's seenKeyFrame should initialize false"
163159
);
164160
assert_eq!(writer.count, 0, "Writer's packet count should initialize 0");
165161
let result = writer.write_rtp(&packet);
166-
if err.is_some() {
167-
assert!(result.is_err(), "{}", msg1);
168-
continue;
169-
} else {
170-
assert!(result.is_ok(), "{}", msg1);
171-
}
162+
163+
assert!(result.is_ok(), "{}", msg1);
172164

173165
assert_eq!(seen_key_frame, writer.seen_key_frame, "{msg1} failed");
174166
if count == 1 {

media/src/io/ivf_writer/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ impl<W: Write + Seek> IVFWriter<W> {
5757
impl<W: Write + Seek> Writer for IVFWriter<W> {
5858
/// write_rtp adds a new packet and writes the appropriate headers for it
5959
fn write_rtp(&mut self, packet: &rtp::packet::Packet) -> Result<()> {
60+
if packet.payload.is_empty() {
61+
return Ok(());
62+
}
63+
6064
let mut depacketizer: Box<dyn Depacketizer> = if self.is_vp9 {
6165
Box::<rtp::codecs::vp9::Vp9Packet>::default()
6266
} else {

media/src/io/ogg_writer/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,10 @@ impl<W: Write + Seek> OggWriter<W> {
169169
impl<W: Write + Seek> Writer for OggWriter<W> {
170170
/// write_rtp adds a new packet and writes the appropriate headers for it
171171
fn write_rtp(&mut self, packet: &rtp::packet::Packet) -> Result<()> {
172+
if packet.payload.is_empty() {
173+
return Ok(());
174+
}
175+
172176
let mut opus_packet = rtp::codecs::opus::OpusPacket;
173177
let payload = opus_packet.depacketize(&packet.payload)?;
174178

media/src/io/ogg_writer/ogg_writer_test.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::io::Cursor;
22

33
use super::*;
4-
use crate::error::Error;
54

65
#[test]
76
fn test_ogg_writer_add_packet_and_close() -> Result<()> {
@@ -36,28 +35,21 @@ fn test_ogg_writer_add_packet_and_close() -> Result<()> {
3635
// nolint:dupl
3736
let add_packet_test_case = vec![
3837
(
39-
"OggWriter shouldn't be able to write an empty packet",
38+
"OggWriter should be able to skip an empty packet",
4039
"OggWriter should be able to close the file",
4140
rtp::packet::Packet::default(),
42-
Some(Error::ErrInvalidNilPacket),
4341
),
4442
(
4543
"OggWriter should be able to write an Opus packet",
4644
"OggWriter should be able to close the file",
4745
valid_packet,
48-
None,
4946
),
5047
];
5148

52-
for (msg1, _msg2, packet, err) in add_packet_test_case {
49+
for (msg1, _msg2, packet) in add_packet_test_case {
5350
let mut writer = OggWriter::new(Cursor::new(Vec::<u8>::new()), 4800, 2)?;
5451
let result = writer.write_rtp(&packet);
55-
if err.is_some() {
56-
assert!(result.is_err(), "{}", msg1);
57-
continue;
58-
} else {
59-
assert!(result.is_ok(), "{}", msg1);
60-
}
52+
assert!(result.is_ok(), "{}", msg1);
6153
writer.close()?;
6254
}
6355

0 commit comments

Comments
 (0)