Fix writer to skip packets with an empty payload (#635)

* Fix writer to skip packets with an empty payload * Fix writer tests
webrtc-rs · Nov 20, 2024 · fc3a0aa · fc3a0aa
1 parent 70fc5b4
commit fc3a0aa
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 24 deletions.
diff --git a/media/src/io/ivf_writer/ivf_writer_test.rs b/media/src/io/ivf_writer/ivf_writer_test.rs
@@ -1,7 +1,6 @@
 use std::io::Cursor;
 
 use super::*;
-use crate::error::Error;
 
 #[test]
 fn test_ivf_writer_add_packet_and_close() -> Result<()> {
@@ -117,26 +116,23 @@ fn test_ivf_writer_add_packet_and_close() -> Result<()> {
 
     let add_packet_test_case = vec![
         (
-            "IVFWriter shouldn't be able to write something an empty packet",
+            "IVFWriter should be able to skip something an empty packet",
             "IVFWriter should be able to close the file",
             rtp::packet::Packet::default(),
-            Some(Error::ErrInvalidNilPacket),
             false,
-            0,
+            1,
         ),
         (
             "IVFWriter should be able to write an IVF packet",
             "IVFWriter should be able to close the file",
             valid_packet.clone(),
-            None,
             false,
             1,
         ),
         (
             "IVFWriter should be able to write a Keframe IVF packet",
             "IVFWriter should be able to close the file",
             keyframe_packet,
-            None,
             true,
             2,
         ),
@@ -155,20 +151,16 @@ fn test_ivf_writer_add_packet_and_close() -> Result<()> {
         unused: 0,                // Unused
     };
 
-    for (msg1, _msg2, packet, err, seen_key_frame, count) in add_packet_test_case {
+    for (msg1, _msg2, packet, seen_key_frame, count) in add_packet_test_case {
         let mut writer = IVFWriter::new(Cursor::new(Vec::<u8>::new()), &header)?;
         assert!(
             !writer.seen_key_frame,
             "Writer's seenKeyFrame should initialize false"
         );
         assert_eq!(writer.count, 0, "Writer's packet count should initialize 0");
         let result = writer.write_rtp(&packet);
-        if err.is_some() {
-            assert!(result.is_err(), "{}", msg1);
-            continue;
-        } else {
-            assert!(result.is_ok(), "{}", msg1);
-        }
+
+        assert!(result.is_ok(), "{}", msg1);
 
         assert_eq!(seen_key_frame, writer.seen_key_frame, "{msg1} failed");
         if count == 1 {

diff --git a/media/src/io/ivf_writer/mod.rs b/media/src/io/ivf_writer/mod.rs
@@ -57,6 +57,10 @@ impl<W: Write + Seek> IVFWriter<W> {
 impl<W: Write + Seek> Writer for IVFWriter<W> {
     /// write_rtp adds a new packet and writes the appropriate headers for it
     fn write_rtp(&mut self, packet: &rtp::packet::Packet) -> Result<()> {
+        if packet.payload.is_empty() {
+            return Ok(());
+        }
+
         let mut depacketizer: Box<dyn Depacketizer> = if self.is_vp9 {
             Box::<rtp::codecs::vp9::Vp9Packet>::default()
         } else {

diff --git a/media/src/io/ogg_writer/mod.rs b/media/src/io/ogg_writer/mod.rs
@@ -169,6 +169,10 @@ impl<W: Write + Seek> OggWriter<W> {
 impl<W: Write + Seek> Writer for OggWriter<W> {
     /// write_rtp adds a new packet and writes the appropriate headers for it
     fn write_rtp(&mut self, packet: &rtp::packet::Packet) -> Result<()> {
+        if packet.payload.is_empty() {
+            return Ok(());
+        }
+
         let mut opus_packet = rtp::codecs::opus::OpusPacket;
         let payload = opus_packet.depacketize(&packet.payload)?;
 

diff --git a/media/src/io/ogg_writer/ogg_writer_test.rs b/media/src/io/ogg_writer/ogg_writer_test.rs
@@ -1,7 +1,6 @@
 use std::io::Cursor;
 
 use super::*;
-use crate::error::Error;
 
 #[test]
 fn test_ogg_writer_add_packet_and_close() -> Result<()> {
@@ -36,28 +35,21 @@ fn test_ogg_writer_add_packet_and_close() -> Result<()> {
     // nolint:dupl
     let add_packet_test_case = vec![
         (
-            "OggWriter shouldn't be able to write an empty packet",
+            "OggWriter should be able to skip an empty packet",
             "OggWriter should be able to close the file",
             rtp::packet::Packet::default(),
-            Some(Error::ErrInvalidNilPacket),
         ),
         (
             "OggWriter should be able to write an Opus packet",
             "OggWriter should be able to close the file",
             valid_packet,
-            None,
         ),
     ];
 
-    for (msg1, _msg2, packet, err) in add_packet_test_case {
+    for (msg1, _msg2, packet) in add_packet_test_case {
         let mut writer = OggWriter::new(Cursor::new(Vec::<u8>::new()), 4800, 2)?;
         let result = writer.write_rtp(&packet);
-        if err.is_some() {
-            assert!(result.is_err(), "{}", msg1);
-            continue;
-        } else {
-            assert!(result.is_ok(), "{}", msg1);
-        }
+        assert!(result.is_ok(), "{}", msg1);
         writer.close()?;
     }