craft-video: Vp9Encoder says a short plane gives an empty frame, and EncodedFrame::is_empty reports it (#913)

crates/video/src/vp9/encode/mod.rs

@@ -79,7 +79,8 @@ impl Default for EncoderConfig {
     }
 }
 
-/// One coded frame.
+/// One coded frame. `data` is empty when nothing was coded: [`Vp9Encoder::encode_frame`] was given
+/// a plane shorter than the configured size (see [`EncodedFrame::is_empty`]).
 #[derive(Clone, Debug)]
 pub struct EncodedFrame {
     pub data: Vec<u8>,
@@ -88,6 +89,14 @@ pub struct EncodedFrame {
     pub filter_level: u8,
 }
 
+impl EncodedFrame {
+    /// True when no frame was coded: the planes given to [`Vp9Encoder::encode_frame`] were shorter
+    /// than the configured size, or the size cannot be addressed. A coded frame is never empty.
+    pub fn is_empty(&self) -> bool {
+        self.data.is_empty()
+    }
+}
+
 /// A plane being coded: source and reconstruction at superblock-aligned size.
 pub(crate) struct Plane {
     /// Stride (superblock-aligned width).
@@ -203,7 +212,9 @@ impl Vp9Encoder {
     }
 
     /// Encode one frame from planar 8-bit 4:2:0 (`y`: width×height, `u`/`v`:
-    /// ((w+1)/2)×((h+1)/2), tightly packed).
+    /// ((w+1)/2)×((h+1)/2), tightly packed). Returns an empty `Vec`, and codes nothing, when a plane
+    /// is shorter than that: check for it, as a short plane is an error in the caller's frame, not
+    /// a gray picture. See [`Vp9Encoder::encode_frame`].
     pub fn encode_yuv420(&mut self, y: &[u8], u: &[u8], v: &[u8]) -> Vec<u8> {
         self.encode_frame(y, u, v, false).data
     }
@@ -270,9 +281,11 @@ impl Vp9Encoder {
     }
 
     /// Encode one frame; `force_key` starts a new key frame. Planes shorter than the configured
-    /// size (`y`: width×height, `u`/`v`: ((w+1)/2)×((h+1)/2)) encode nothing: the result has empty
-    /// `data`, the encoder's state is unchanged, and no coding planes are allocated or filled
-    /// from samples that are not there.
+    /// size (`y`: width×height, `u`/`v`: ((w+1)/2)×((h+1)/2)) encode nothing and report no error:
+    /// the result has empty `data` ([`EncodedFrame::is_empty`] is true), the encoder's state is
+    /// unchanged (the next frame is still due as a key frame if this one was), and no coding planes
+    /// are allocated or filled from samples that are not there. Callers must check for the empty
+    /// frame and not mux it.
     pub fn encode_frame(&mut self, y: &[u8], u: &[u8], v: &[u8], force_key: bool) -> EncodedFrame {
         let (w, h) = (self.cfg.width.max(1) as usize, self.cfg.height.max(1) as usize);
         let empty = || EncodedFrame { data: Vec::new(), key: false, q_idx: 0, filter_level: 0 };
@@ -514,12 +527,13 @@ mod tests {
     #[test]
     fn planes_shorter_than_the_frame_encode_nothing() {
         let mut enc = Vp9Encoder::new(EncoderConfig { width: 65_536, height: 65_536, ..Default::default() });
-        assert!(enc.encode_frame(&[1, 2, 3], &[], &[], true).data.is_empty());
+        assert!(enc.encode_frame(&[1, 2, 3], &[], &[], true).is_empty());
+        assert!(enc.encode_yuv420(&[1, 2, 3], &[], &[]).is_empty());
         let mut enc = Vp9Encoder::new(EncoderConfig { width: 16, height: 16, ..Default::default() });
         for (y, u, v) in [(255, 64, 64), (256, 63, 64), (256, 64, 63)] {
             assert!(enc.encode_frame(&vec![128; y], &vec![128; u], &vec![128; v], true).data.is_empty());
         }
-        assert!(!enc.encode_frame(&[128; 256], &[128; 64], &[128; 64], true).data.is_empty());
+        assert!(!enc.encode_frame(&[128; 256], &[128; 64], &[128; 64], true).is_empty());
     }
 
     /// A 1080p stream is above the 983,040 luma samples of level 3.1, so it must not claim 3.1 in