fix clippy & enhance CI/CD (#78)

* fix clippy::unused_io_amount See related clippy documentation, but in short some partial reads can occur in particular with io on the networl. read_exact/write_all transparently handle such errors. The fix actually revealed a bug in 'mp4a' atom parsing, but this is a dangerous change: Parsing bugs that were transparently ignored are now failing with error (unattended io eof). * fix trivial clippy errors * fix clippy error with always 0 value * run ci/cd with clippy and latest rust version
alfg · Jul 8, 2022 · 5d648f1 · 5d648f1
1 parent 00385ba
commit 5d648f1
Show file tree

Hide file tree

Showing 13 changed files with 90 additions and 86 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -13,8 +13,38 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
-    - name: Build
-      run: cargo build --verbose
-    - name: Run tests
-      run: cargo test --verbose
+      - name: Checkout sources
+        uses: actions/checkout@v2
+
+      - name: Install rust toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: stable
+          override: true
+          components: rustfmt, clippy
+
+      - name: Setup rust smart caching
+        uses: Swatinem/rust-cache@v1.3.0
+
+      - name: Cargo fmt
+        uses: actions-rs/cargo@v1
+        with:
+          command: fmt
+          args: --all -- --check
+
+      - name: Cargo clippy
+        uses: actions-rs/cargo@v1
+        with:
+          command: clippy
+          args: --no-deps -- -D warnings
+
+      - name: Cargo build
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+
+      - name: Cargo test
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
diff --git a/examples/mp4dump.rs b/examples/mp4dump.rs
@@ -45,12 +45,11 @@ fn get_boxes(file: File) -> Result<Vec<Box>> {
     let mp4 = mp4::Mp4Reader::read_header(reader, size)?;
 
     // collect known boxes
-    let mut boxes = Vec::new();
-
-    // ftyp, moov, mvhd
-    boxes.push(build_box(&mp4.ftyp));
-    boxes.push(build_box(&mp4.moov));
-    boxes.push(build_box(&mp4.moov.mvhd));
+    let mut boxes = vec![
+        build_box(&mp4.ftyp),
+        build_box(&mp4.moov),
+        build_box(&mp4.moov.mvhd),
+    ];
 
     if let Some(ref mvex) = &mp4.moov.mvex {
         boxes.push(build_box(mvex));

diff --git a/src/mp4box/avc1.rs b/src/mp4box/avc1.rs
@@ -283,13 +283,13 @@ impl NalUnit {
     fn read<R: Read + Seek>(reader: &mut R) -> Result<Self> {
         let length = reader.read_u16::<BigEndian>()? as usize;
         let mut bytes = vec![0u8; length];
-        reader.read(&mut bytes)?;
+        reader.read_exact(&mut bytes)?;
         Ok(NalUnit { bytes })
     }
 
     fn write<W: Write>(&self, writer: &mut W) -> Result<u64> {
         writer.write_u16::<BigEndian>(self.bytes.len() as u16)?;
-        writer.write(&self.bytes)?;
+        writer.write_all(&self.bytes)?;
         Ok(self.size() as u64)
     }
 }

diff --git a/src/mp4box/dinf.rs b/src/mp4box/dinf.rs
@@ -289,7 +289,7 @@ impl<W: Write> WriteBox<&mut W> for UrlBox {
         write_box_header_ext(writer, self.version, self.flags)?;
 
         if !self.location.is_empty() {
-            writer.write(self.location.as_bytes())?;
+            writer.write_all(self.location.as_bytes())?;
             writer.write_u8(0)?;
         }
 

diff --git a/src/mp4box/edts.rs b/src/mp4box/edts.rs
@@ -55,12 +55,9 @@ impl<R: Read + Seek> ReadBox<&mut R> for EdtsBox {
         let header = BoxHeader::read(reader)?;
         let BoxHeader { name, size: s } = header;
 
-        match name {
-            BoxType::ElstBox => {
-                let elst = ElstBox::read_box(reader, s)?;
-                edts.elst = Some(elst);
-            }
-            _ => {}
+        if let BoxType::ElstBox = name {
+            let elst = ElstBox::read_box(reader, s)?;
+            edts.elst = Some(elst);
         }
 
         skip_bytes_to(reader, start + size)?;

diff --git a/src/mp4box/hdlr.rs b/src/mp4box/hdlr.rs
@@ -92,7 +92,7 @@ impl<W: Write> WriteBox<&mut W> for HdlrBox {
             writer.write_u32::<BigEndian>(0)?;
         }
 
-        writer.write(self.name.as_bytes())?;
+        writer.write_all(self.name.as_bytes())?;
         writer.write_u8(0)?;
 
         Ok(size)

diff --git a/src/mp4box/mehd.rs b/src/mp4box/mehd.rs
@@ -4,7 +4,7 @@ use std::io::{Read, Seek, Write};
 
 use crate::mp4box::*;
 
-#[derive(Debug, Clone, PartialEq, Serialize)]
+#[derive(Debug, Clone, PartialEq, Serialize, Default)]
 pub struct MehdBox {
     pub version: u8,
     pub flags: u32,
@@ -28,16 +28,6 @@ impl MehdBox {
     }
 }
 
-impl Default for MehdBox {
-    fn default() -> Self {
-        MehdBox {
-            version: 0,
-            flags: 0,
-            fragment_duration: 0,
-        }
-    }
-}
-
 impl Mp4Box for MehdBox {
     fn box_type(&self) -> BoxType {
         self.get_type()

diff --git a/src/mp4box/mod.rs b/src/mp4box/mod.rs
@@ -122,9 +122,9 @@ macro_rules! boxtype {
             }
         }
 
-        impl Into<u32> for BoxType {
-            fn into(self) -> u32 {
-                match self {
+        impl From<BoxType> for u32 {
+            fn from(b: BoxType) -> u32 {
+                match b {
                     $( BoxType::$name => $value, )*
                     BoxType::UnknownBox(t) => t,
                 }
@@ -212,7 +212,7 @@ impl BoxHeader {
     pub fn read<R: Read>(reader: &mut R) -> Result<Self> {
         // Create and read to buf.
         let mut buf = [0u8; 8]; // 8 bytes for box header.
-        reader.read(&mut buf)?;
+        reader.read_exact(&mut buf)?;
 
         // Get size.
         let s = buf[0..4].try_into().unwrap();
@@ -224,9 +224,8 @@ impl BoxHeader {
 
         // Get largesize if size is 1
         if size == 1 {
-            reader.read(&mut buf)?;
-            let s = buf.try_into().unwrap();
-            let largesize = u64::from_be_bytes(s);
+            reader.read_exact(&mut buf)?;
+            let largesize = u64::from_be_bytes(buf);
 
             Ok(BoxHeader {
                 name: BoxType::from(typ),

diff --git a/src/mp4box/mp4a.rs b/src/mp4box/mp4a.rs
@@ -89,14 +89,17 @@ impl<R: Read + Seek> ReadBox<&mut R> for Mp4aBox {
         reader.read_u32::<BigEndian>()?; // pre-defined, reserved
         let samplerate = FixedPointU16::new_raw(reader.read_u32::<BigEndian>()?);
 
-        let header = BoxHeader::read(reader)?;
-        let BoxHeader { name, size: s } = header;
-
         let mut esds = None;
-        if name == BoxType::EsdsBox {
-            esds = Some(EsdsBox::read_box(reader, s)?);
+        let current = reader.seek(SeekFrom::Current(0))?;
+        if current < start + size {
+            let header = BoxHeader::read(reader)?;
+            let BoxHeader { name, size: s } = header;
+
+            if name == BoxType::EsdsBox {
+                esds = Some(EsdsBox::read_box(reader, s)?);
+            }
+            skip_bytes_to(reader, start + size)?;
         }
-        skip_bytes_to(reader, start + size)?;
 
         Ok(Mp4aBox {
             data_reference_index,

diff --git a/src/mp4box/tfhd.rs b/src/mp4box/tfhd.rs
@@ -4,25 +4,14 @@ use std::io::{Read, Seek, Write};
 
 use crate::mp4box::*;
 
-#[derive(Debug, Clone, PartialEq, Serialize)]
+#[derive(Debug, Clone, PartialEq, Serialize, Default)]
 pub struct TfhdBox {
     pub version: u8,
     pub flags: u32,
     pub track_id: u32,
     pub base_data_offset: u64,
 }
 
-impl Default for TfhdBox {
-    fn default() -> Self {
-        TfhdBox {
-            version: 0,
-            flags: 0,
-            track_id: 0,
-            base_data_offset: 0,
-        }
-    }
-}
-
 impl TfhdBox {
     pub fn get_type(&self) -> BoxType {
         BoxType::TfhdBox

diff --git a/src/track.rs b/src/track.rs
@@ -502,10 +502,7 @@ impl Mp4Track {
         }
 
         if let Some(ref stss) = self.trak.mdia.minf.stbl.stss {
-            match stss.entries.binary_search(&sample_id) {
-                Ok(_) => true,
-                Err(_) => false,
-            }
+            stss.entries.binary_search(&sample_id).is_ok()
         } else {
             true
         }

diff --git a/src/types.rs b/src/types.rs
@@ -206,9 +206,9 @@ impl TryFrom<&FourCC> for TrackType {
     }
 }
 
-impl Into<FourCC> for TrackType {
-    fn into(self) -> FourCC {
-        match self {
+impl From<TrackType> for FourCC {
+    fn from(t: TrackType) -> FourCC {
+        match t {
             TrackType::Video => HANDLER_TYPE_VIDEO_FOURCC.into(),
             TrackType::Audio => HANDLER_TYPE_AUDIO_FOURCC.into(),
             TrackType::Subtitle => HANDLER_TYPE_SUBTITLE_FOURCC.into(),
@@ -252,9 +252,9 @@ impl TryFrom<&str> for MediaType {
     }
 }
 
-impl Into<&str> for MediaType {
-    fn into(self) -> &'static str {
-        match self {
+impl From<MediaType> for &str {
+    fn from(t: MediaType) -> &'static str {
+        match t {
             MediaType::H264 => MEDIA_TYPE_H264,
             MediaType::H265 => MEDIA_TYPE_H265,
             MediaType::VP9 => MEDIA_TYPE_VP9,
@@ -264,9 +264,9 @@ impl Into<&str> for MediaType {
     }
 }
 
-impl Into<&str> for &MediaType {
-    fn into(self) -> &'static str {
-        match self {
+impl From<&MediaType> for &str {
+    fn from(t: &MediaType) -> &'static str {
+        match t {
             MediaType::H264 => MEDIA_TYPE_H264,
             MediaType::H265 => MEDIA_TYPE_H265,
             MediaType::VP9 => MEDIA_TYPE_VP9,
@@ -290,7 +290,7 @@ impl TryFrom<(u8, u8)> for AvcProfile {
     type Error = Error;
     fn try_from(value: (u8, u8)) -> Result<AvcProfile> {
         let profile = value.0;
-        let constraint_set1_flag = value.1 & 0x40 >> 7;
+        let constraint_set1_flag = (value.1 & 0x40) >> 7;
         match (profile, constraint_set1_flag) {
             (66, 1) => Ok(AvcProfile::AvcConstrainedBaseline),
             (66, 0) => Ok(AvcProfile::AvcBaseline),
@@ -503,20 +503,20 @@ impl TryFrom<u8> for SampleFreqIndex {
 
 impl SampleFreqIndex {
     pub fn freq(&self) -> u32 {
-        match self {
-            &SampleFreqIndex::Freq96000 => 96000,
-            &SampleFreqIndex::Freq88200 => 88200,
-            &SampleFreqIndex::Freq64000 => 64000,
-            &SampleFreqIndex::Freq48000 => 48000,
-            &SampleFreqIndex::Freq44100 => 44100,
-            &SampleFreqIndex::Freq32000 => 32000,
-            &SampleFreqIndex::Freq24000 => 24000,
-            &SampleFreqIndex::Freq22050 => 22050,
-            &SampleFreqIndex::Freq16000 => 16000,
-            &SampleFreqIndex::Freq12000 => 12000,
-            &SampleFreqIndex::Freq11025 => 11025,
-            &SampleFreqIndex::Freq8000 => 8000,
-            &SampleFreqIndex::Freq7350 => 7350,
+        match *self {
+            SampleFreqIndex::Freq96000 => 96000,
+            SampleFreqIndex::Freq88200 => 88200,
+            SampleFreqIndex::Freq64000 => 64000,
+            SampleFreqIndex::Freq48000 => 48000,
+            SampleFreqIndex::Freq44100 => 44100,
+            SampleFreqIndex::Freq32000 => 32000,
+            SampleFreqIndex::Freq24000 => 24000,
+            SampleFreqIndex::Freq22050 => 22050,
+            SampleFreqIndex::Freq16000 => 16000,
+            SampleFreqIndex::Freq12000 => 12000,
+            SampleFreqIndex::Freq11025 => 11025,
+            SampleFreqIndex::Freq8000 => 8000,
+            SampleFreqIndex::Freq7350 => 7350,
         }
     }
 }

diff --git a/tests/lib.rs b/tests/lib.rs
@@ -24,7 +24,7 @@ fn test_read_mp4() {
 
     for b in brands {
         let t = mp4.compatible_brands().iter().any(|x| x.to_string() == b);
-        assert_eq!(t, true);
+        assert!(t);
     }
 
     assert_eq!(mp4.duration(), Duration::from_millis(62));