replace new_parameters with has_new_parameters

As described in #47

examples/client/mp4.rs

@@ -550,9 +550,9 @@ impl<W: AsyncWrite + AsyncSeek + Send + Unpin> Mp4Writer<W> {
             &frame.timestamp(),
             frame.data().remaining(),
         );
-        let sample_description_index = if let (Some(i), None) = (
+        let sample_description_index = if let (Some(i), true) = (
             self.cur_video_params_sample_description_index,
-            frame.new_parameters.as_ref(),
+            frame.has_new_parameters(),
         ) {
             // Use the most recent sample description index for most frames, without having to
             // scan through self.video_sample_index.

src/client/mod.rs

@@ -780,13 +780,12 @@ impl Stream {
     ///
     /// # With [`Demuxed`]
     ///
-    /// When using [`Demuxed`]'s frame-by-frame `futures::Stream` impl:
+    /// When using [`Demuxed`]'s frame-by-frame `futures::Stream` impl, `parameters` reflects
+    /// all parameters as of returned frames that have been returned from from `poll_next` via
+    /// `Poll::Ready`.
     ///
-    /// *   After `poll_next` returns `Ready`, `parameters` reflects all parameter changes as of
+    /// It's guaranteed to *not* reflect any parameter changes in the upcoming frame, even after
-    ///     returned frame.
+    /// a `Poll::Pending` return.
-    /// *   After `poll_next` returns `Pending`, currently the parameters may or may not reflect
-    ///     changes sent as part of the *next* frame that `poll_next` will return. (It's likely
-    ///     that an upcoming Retina release will guarantee not.)
     ///
     /// If there is no packet loss, parameters are generally available after the first frame is
     /// returned. In the case of H.264, [RFC 6184 section
@@ -2357,7 +2356,16 @@ impl futures::Stream for Demuxed {
                         description,
                     })
                 })?;
+
+                // Note we're committed now to calling `pull` and returning
+                // `Ready` if it has a frame. This is because the
+                // `Stream::parameters` contract guarantees that changes in
+                // upcoming frames are *not* reflected. It's implemented by a
+                // call to `Depacketizer::pull`, which doesn't make a like
+                // guarantee about the state between `push` and `pull`. So we
+                // can't let our callers observe that state.
             }
+
             match depacketizer.pull(conn_ctx, stream_ctx) {
                 Ok(Some(item)) => {
                     self.state = DemuxedState::Pulling(stream_id);

src/codec/h264.rs

@@ -496,31 +496,27 @@ impl Depacketizer {
         self.nals.clear();
         self.pieces.clear();
 
-        let new_parameters = match (
+        let has_new_parameters = match (
             new_sps.as_deref(),
             new_pps.as_deref(),
             self.parameters.as_ref(),
         ) {
             (Some(sps_nal), Some(pps_nal), _) => {
                 // TODO: could map this to a RtpPacketError more accurately.
-                let ip = InternalParameters::parse_sps_and_pps(sps_nal, pps_nal)?;
+                self.parameters = Some(InternalParameters::parse_sps_and_pps(sps_nal, pps_nal)?);
-                let p = ip.generic_parameters.clone();
+                true
-                self.parameters = Some(ip);
-                Some(Box::new(p))
             }
             (Some(_), None, Some(old_ip)) | (None, Some(_), Some(old_ip)) => {
                 let sps_nal = new_sps.as_deref().unwrap_or(&old_ip.sps_nal);
                 let pps_nal = new_pps.as_deref().unwrap_or(&old_ip.pps_nal);
                 // TODO: as above, could map this to a RtpPacketError more accurately.
-                let ip = InternalParameters::parse_sps_and_pps(sps_nal, pps_nal)?;
+                self.parameters = Some(InternalParameters::parse_sps_and_pps(sps_nal, pps_nal)?);
-                let p = ip.generic_parameters.clone();
+                true
-                self.parameters = Some(ip);
-                Some(Box::new(p))
             }
-            _ => None,
+            _ => false,
         };
         Ok(VideoFrame {
-            new_parameters,
+            has_new_parameters,
             loss: au.loss,
             start_ctx: au.start_ctx,
             end_ctx: au.end_ctx,
@@ -1429,9 +1425,7 @@ mod tests {
         };
 
         // After pull, new_parameters and parameters() both reflect the change.
-        assert!(frame.new_parameters.is_some());
+        assert!(frame.has_new_parameters);
-        let p = frame.new_parameters.unwrap();
-        assert_eq!(p.pixel_dimensions(), (640, 480));
         match d.parameters() {
             Some(crate::codec::Parameters::Video(v)) => {
                 assert_eq!(v.pixel_dimensions(), (640, 480));
@@ -1538,7 +1532,7 @@ mod tests {
             Some(CodecItem::VideoFrame(frame)) => frame,
             _ => panic!(),
         };
-        assert!(frame.new_parameters.is_some());
+        assert!(frame.has_new_parameters);
         assert!(d.parameters().is_some());
     }
 }

src/codec/mod.rs

@@ -324,16 +324,8 @@ pub struct VideoFrame {
     start_ctx: crate::PacketContext,
     end_ctx: crate::PacketContext,
 
-    /// New video parameters.
+    has_new_parameters: bool,
-    ///
-    /// Rarely populated and large, so boxed to reduce bloat.
-    //
-    /// To obtain the current parameters for the stream regardless of whether this frame set new
-    /// parameters, see [`crate::client::Stream::parameters`].
-    pub new_parameters: Option<Box<VideoParameters>>,
-
     loss: u16,
-
     timestamp: crate::Timestamp,
     stream_id: usize,
     is_random_access_point: bool,
@@ -347,6 +339,14 @@ impl VideoFrame {
         self.stream_id
     }
 
+    /// Returns true if this frame set new video parameters.
+    ///
+    /// The parameters can be obtained via [`crate::client::Stream::parameters`].
+    #[inline]
+    pub fn has_new_parameters(&self) -> bool {
+        self.has_new_parameters
+    }
+
     /// Returns the number of lost RTP packets before this video frame. See
     /// [`crate::rtp::ReceivedPacket::loss`].
     ///
@@ -418,7 +418,7 @@ impl std::fmt::Debug for VideoFrame {
             .field("start_ctx", &self.start_ctx)
             .field("end_ctx", &self.end_ctx)
             .field("loss", &self.loss)
-            .field("new_parameters", &self.new_parameters)
+            .field("has_new_parameters", &self.has_new_parameters)
             .field("is_random_access_point", &self.is_random_access_point)
             .field("is_disposable", &self.is_disposable)
             .field("data", &crate::hex::LimitedHex::new(&self.data, 64))