From 013f63a23e5e2d3160239160848df9ae7c4e9ecc Mon Sep 17 00:00:00 2001 From: JasonLG1979 Date: Thu, 22 Jun 2023 14:29:04 -0500 Subject: [PATCH] Update notify_about_position logic It would be so much easier to use elapsed but elapsed could potentially panic is rare cases. See: https://doc.rust-lang.org/std/time/struct.Instant.html#monotonicity Otherwise this is pretty straight forward. If anything fails getting expected_position_ms it will return 0 which will trigger a notify if either stream_position_ms or decoder_position_ms >= 1000. If all goes well it's simply a matter of calculating the max delta of expected_position_ms and stream_position_ms and expected_position_ms and decoder_position_ms. So if the decoder or the sample pipeline are off by more than 1 sec we notify. --- playback/src/player.rs | 90 +++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 50 deletions(-) diff --git a/playback/src/player.rs b/playback/src/player.rs index bf2b915cf..d2c3fe00f 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -1150,68 +1150,58 @@ impl Future for PlayerInternal { match decoder.next_packet() { Ok(result) => { if let Some((ref packet_position, ref packet)) = result { - let new_stream_position_ms = packet_position - .position_ms - .saturating_sub(sample_pipeline_latency_ms); + let decoder_position_ms = packet_position.position_ms; - let expected_position_ms = std::mem::replace( - &mut *stream_position_ms, - new_stream_position_ms, - ); + *stream_position_ms = + decoder_position_ms.saturating_sub(sample_pipeline_latency_ms); if !passthrough { match packet.samples() { Ok(_) => { - let new_stream_position = Duration::from_millis( - new_stream_position_ms as u64, - ); - let now = Instant::now(); - // Only notify if we're skipped some packets *or* we are behind. - // If we're ahead it's probably due to a buffer of the backend - // and we're actually in time. - let notify_about_position = - match *reported_nominal_start_time { - None => true, - Some(reported_nominal_start_time) => { - let mut notify = false; - - if packet_position.skipped { - if let Some(ahead) = new_stream_position - .checked_sub(Duration::from_millis( - expected_position_ms as u64, - )) - { - notify |= - ahead >= Duration::from_secs(1) - } - } - - if let Some(lag) = now - .checked_duration_since( - reported_nominal_start_time, - ) - { - if let Some(lag) = - lag.checked_sub(new_stream_position) - { - notify |= - lag >= Duration::from_secs(1) - } - } - - notify || sample_pipeline_latency_ms > 1000 - } - }; + let notify_about_position = { + // It would be so much easier to use elapsed but elapsed could + // potentially panic is rare cases. + // See: + // https://doc.rust-lang.org/std/time/struct.Instant.html#monotonicity + // + // Otherwise this is pretty straight forward. If anything fails getting + // expected_position_ms it will return 0 which will trigger a notify if + // either stream_position_ms or decoder_position_ms >= 1000. If all goes + // well it's simply a matter of calculating the max delta of expected_position_ms + // and stream_position_ms and expected_position_ms and decoder_position_ms. + // So if the decoder or the sample pipeline are off by more than 1 sec we notify. + let expected_position_ms = now + .checked_duration_since( + reported_nominal_start_time.unwrap_or(now), + ) + .unwrap_or(Duration::ZERO) + .as_millis(); + + let max_expected_position_delta_ms = + expected_position_ms + .abs_diff(*stream_position_ms as u128) + .max( + expected_position_ms.abs_diff( + decoder_position_ms as u128, + ), + ); + + max_expected_position_delta_ms > 1000 + }; if notify_about_position { - *reported_nominal_start_time = - now.checked_sub(new_stream_position); + let position_ms = *stream_position_ms; + + *reported_nominal_start_time = now.checked_sub( + Duration::from_millis(position_ms as u64), + ); + self.send_event(PlayerEvent::PositionCorrection { play_request_id, track_id, - position_ms: new_stream_position_ms, + position_ms, }); } }