🐛 bug: popping is gone

It was a combination of two bugs:

1. For some reason, libswresample sometimes returns more data than you need.
Like, it'll return X samples but _more than_ 8X bytes, for f32 stereo.

2. I wasn't supposed to be calling `flush`. I should have been calling
`run` with a dummy frame. This isn't intuitive to me, and it required
me to construct a dummy frame and keep it around in my code. I think
this is pretty inelegant. And looking at the ffmpeg code, I think it's
a flaw in the API design of the `ffmpeg_next` crate. I may ask them
about it in the future.

.gitignore

@@ -1,4 +1,5 @@
 /target
+*.data
 *.m4a
 *.ogg
 *.opus

src/decoder.rs

@@ -24,6 +24,7 @@ use ffmpeg_next::{
 			self,
 			Sample,
 		},
+		frame::Audio as AudioFrame,
 	},
 };
 
@@ -83,10 +84,11 @@ pub struct SharedState {
 
 pub struct Decoder {
 	pub input_ctx: DemuxContext,
-	best_stream_idx: usize,
 	pub decoder: DecodeContext,
 	pub resampler: ResamplingContext,
-	resampler_flushed: bool,
+	
+	best_stream_idx: usize,
+	dummy_frame: Option <AudioFrame>,
 }
 
 impl Decoder {
@@ -107,49 +109,124 @@ impl Decoder {
 		
 		Ok (Self {
 			input_ctx,
-			best_stream_idx,
 			decoder,
 			resampler,
-			resampler_flushed: false,
+			
+			best_stream_idx,
+			dummy_frame: None,
 		})
 	}
 	
-	pub fn fill_buffer (&mut self, pcm_buffers: &mut PcmBuffers) -> Result <bool> {
-		if ! self.resampler_flushed && self.resampler.delay ().is_some () {
-			let mut frame_resampled = ffmpeg_next::util::frame::Audio::empty ();
-			frame_resampled.set_channel_layout (ChannelLayout::STEREO);
-			frame_resampled.set_format (Sample::F32 (sample::Type::Packed));
-			frame_resampled.set_rate (48000);
+	fn new_frame () -> AudioFrame {
+		let mut x = AudioFrame::empty ();
+		x.set_channel_layout (ChannelLayout::STEREO);
+		x.set_format (Sample::F32 (sample::Type::Packed));
+		x
+	}
+	
+	pub fn next (&mut self) -> Result <Option <AudioFrame>> {
+		Ok (if let Some (frame) = self.pump ()? {
+			assert_eq! (frame.rate (), 48000);
+			assert! (frame.samples () > 0);
 			
-			let rc = self.resampler.flush (&mut frame_resampled)?;
-			if rc.is_none () {
-				self.resampler_flushed = true;
+			let actual_bytes = frame.data (0).len ();
+			let expected_bytes = frame.samples () * 4 * 2;
+			
+			assert! (actual_bytes >= expected_bytes);
+			
+			if actual_bytes > expected_bytes {
+				let extra_bytes = actual_bytes - expected_bytes;
+				let extra_samples = extra_bytes / 4 / 2;
+				// tracing::debug! ("Extra bytes: {}", extra_bytes);
+				// tracing::debug! ("Extra samples: {}", extra_samples);
 			}
 			
-			if ! frame_resampled.data (0).is_empty () {
-				tracing::trace! ("flushing resampler... {}", frame_resampled.data (0).len ());
-				pcm_buffers.produce_bytes (frame_resampled.data (0));
-				
-				return Ok (true);
+			Some (frame)
+		}
+		else {
+			None
+		})
+	}
+	
+	fn pump (&mut self) -> Result <Option <AudioFrame>> {
+		loop {
+			match self.pump_resampler ()? {
+				Some (x) => {
+					// tracing::trace! ("Pumped resampler");
+					return Ok (Some (x));
+				},
+				None => (),
+			}
+			
+			match self.pump_decoder ()? {
+				Some (x) => {
+					// tracing::trace! ("Pumped decoder");
+					return Ok (Some (x));
+				},
+				None => (),
+			}
+			
+			if self.pump_demuxer ()? {
+				// tracing::trace! ("Pumped demuxer");
+				continue;
+			}
+			else {
+				break;
 			}
 		}
 		
-		let mut frame_src = ffmpeg_next::util::frame::Audio::empty ();
-		if self.decoder.receive_frame (&mut frame_src).is_ok () {
-			// tracing::trace! ("decoded frame");
-			let mut frame_resampled = ffmpeg_next::util::frame::Audio::empty ();
-			frame_resampled.set_channel_layout (ChannelLayout::STEREO);
-			frame_resampled.set_format (Sample::F32 (sample::Type::Packed));
-			frame_resampled.set_rate (48000);
-			
-			self.resampler.run (&frame_src, &mut frame_resampled)?;
-			self.resampler_flushed = false;
-			pcm_buffers.produce_bytes (frame_resampled.data (0));
-			return Ok (true);
+		Ok (None)
+	}
+	
+	pub fn pump_resampler (&mut self) -> Result <Option <AudioFrame>> {
+		let frame_src = match self.dummy_frame.as_ref () {
+			None => return Ok (None),
+			Some (x) => x,
+		};
+		
+		let mut frame_resampled = Self::new_frame ();
+		
+		let _rc = self.resampler.run (&frame_src, &mut frame_resampled)?;
+		// dbg! (&frame_resampled, rc);
+		
+		Ok (if frame_resampled.samples () > 0 {
+			// tracing::trace! ("Pulled from resampler FIFO");
+			Some (frame_resampled)
+		}
+		else {
+			None
+		})
+	}
+	
+	pub fn pump_decoder (&mut self) -> Result <Option <AudioFrame>> {
+		let mut frame_src = AudioFrame::empty ();
+		if let Err (_) = self.decoder.receive_frame (&mut frame_src) {
+			return Ok (None);
+		};
+		
+		if self.dummy_frame.is_none () {
+			let mut dummy_frame = AudioFrame::new (
+				frame_src.format (),
+				0,
+				frame_src.channel_layout (),
+			);
+			dummy_frame.set_rate (frame_src.rate ());
+			self.dummy_frame = Some (dummy_frame);
 		}
 		
-		//eprintln! ("Decoder is dry, fetching a new packet...");
+		let nb_output_samples = frame_src.samples ();
 		
+		let mut frame_resampled = AudioFrame::new (
+			Sample::F32 (sample::Type::Packed),
+			nb_output_samples,
+			ChannelLayout::STEREO
+		);
+		
+		self.resampler.run (&frame_src, &mut frame_resampled)?;
+		Ok (Some (frame_resampled))
+	}
+	
+	pub fn pump_demuxer (&mut self) -> Result <bool> {
 		while let Some ((stream, packet)) = self.input_ctx.packets ().next () {
 			if stream.index () == self.best_stream_idx {
 				// tracing::trace! ("demuxed packet");
@@ -158,27 +235,6 @@ impl Decoder {
 			}
 		}
 		
-		if self.resampler_flushed {
-			return Ok (false);
-		}
-		
-		let mut frame_resampled = ffmpeg_next::util::frame::Audio::empty ();
-		frame_resampled.set_channel_layout (ChannelLayout::STEREO);
-		frame_resampled.set_format (Sample::F32 (sample::Type::Packed));
-		frame_resampled.set_rate (48000);
-		
-		let rc = self.resampler.flush (&mut frame_resampled)?;
-		if rc.is_none () {
-			self.resampler_flushed = true;
-		}
-		
-		if ! frame_resampled.data (0).is_empty () {
-			tracing::trace! ("flushing resampler... {}", frame_resampled.data (0).len ());
-			pcm_buffers.produce_bytes (frame_resampled.data (0));
-			
-			return Ok (true);
-		}
-		
 		Ok (false)
 	}
 }

src/main.rs

@@ -19,11 +19,6 @@ use anyhow::{
 	Result,
 };
 
-use byteorder::{
-	ByteOrder,
-	LittleEndian,
-};
-
 use cpal::traits::{
 	DeviceTrait,
 	HostTrait,
@@ -36,13 +31,14 @@ fn main () -> Result <()> {
 	
 	match args.get (1).map (|s| &s[..]) {
 		None => bail! ("First argument must be a subcommand like `play`"),
-		Some ("debug") => cmd_debug (&args [1..]),
+		Some ("debug-dump") => cmd_debug_dump (&args [1..]),
+		Some ("debug-pipe") => cmd_debug_pipe (&args [1..]),
 		Some ("play") => cmd_play (&args [1..]),
 		Some (_) => bail! ("Unrecognized subcommand"),
 	}
 }
 
-fn cmd_debug (args: &[String]) -> Result <()> {
+fn cmd_debug_dump (args: &[String]) -> Result <()> {
 	tracing_subscriber::fmt::init ();
 	
 	let filename = args.get (1)
@@ -50,33 +46,38 @@ fn cmd_debug (args: &[String]) -> Result <()> {
 	.unwrap_or_else (|| "test-short.m4a".to_string ());
 	
 	let mut decoder = decoder::Decoder::new (&filename)?;
-	let mut pcm_buffers = decoder::PcmBuffers::default ();
 	
 	tracing::debug! ("Decoding...");
 	
 	let mut f = File::create ("pcm-dump.data")?;
 	
-	'decoding: loop {
-		while pcm_buffers.samples_available () < 12_000 {
-			if ! decoder.fill_buffer (&mut pcm_buffers)? {
-				tracing::info! ("Decoder finished");
-				break 'decoding;
-			}
-		}
-		
-		while pcm_buffers.samples_available () > 0 {
-			let mut src = vec! [0.0f32; pcm_buffers.samples_available ()];
-			// dbg! (src.len ());
-			pcm_buffers.consume_exact (&mut src);
-			let mut dest = vec! [0; src.len () * 4];
-			LittleEndian::write_f32_into (&src, &mut dest);
-			
-			f.write_all (&dest)?;
-		}
+	while let Some (frame) = decoder.next ()? {
+		f.write_all (&frame.data (0) [0..frame.samples () * 4 * 2])?;
+		// f.write_all (frame.data (0))?;
 	}
 	
 	Ok (())
 }
+
+fn cmd_debug_pipe (args: &[String]) -> Result <()> {
+	tracing_subscriber::fmt::init ();
+	
+	let filename = args.get (1)
+	.map (|s| s.to_string ())
+	.unwrap_or_else (|| "test-short.m4a".to_string ());
+	
+	let mut decoder = decoder::Decoder::new (&filename)?;
+	let mut sample_count = 0;
+	
+	while let Some (frame) = decoder.next ()? {
+		sample_count += frame.samples ();
+		// dbg! (frame, sample_count);
+	}
+	
+	dbg! (sample_count);
+	
+	Ok (())
+}
 	
 fn cmd_play (args: &[String]) -> Result <()> {
 	tracing_subscriber::fmt::init ();
@@ -114,9 +115,12 @@ fn cmd_play (args: &[String]) -> Result <()> {
 				
 				while pcm_buffers.samples_available () < 12_000 {
 					// tracing::trace! ("Decoder is trying to work...");
-					if ! decoder.fill_buffer (pcm_buffers)? {
-						tracing::info! ("Finished decoding file");
-						break 'one_file;
+					match decoder.next ()? {
+						Some (frame) => pcm_buffers.produce_bytes (&frame.data (0) [0..frame.samples () * 4 * 2]),
+						None => {
+							tracing::info! ("Finished decoding file");
+							break 'one_file;
+						}
 					}
 				}
 			}
@@ -151,7 +155,7 @@ fn cmd_play (args: &[String]) -> Result <()> {
 			};
 			let time_stop = Instant::now ();
 			let dur = time_stop - time_start;
-			if dur.as_micros () > 1 {
+			if dur.as_micros () > 2 {
 				dbg! (dur.as_micros ());
 			}
 			

todo.md

@@ -1,3 +1 @@
 # Critical
-
-- Popping every 1-3 seconds-ish