Feature: Guess notes from audio file (redo) #3
Conversation
7b7e0cf to
1e8bfef
Compare
2a1799c to
dcdce8f
Compare
src/analyze.rs
Outdated
| println!("{notes:?}"); | ||
| assert!(notes.contains(&CFive)); | ||
| assert!(notes.contains(&EFlatSeven)); | ||
| assert!(notes.contains(&GEight)); |
There was a problem hiding this comment.
Can we get an audio file that is a little more straightforward? Like something that is a regular Cm7b9, or something like that?
Mostly concerned about that G8 coming though. Do you know why that G8 is there?
There was a problem hiding this comment.
Sorry, I just grabbed a sample from some pack on the internet. Shouldn't probably do that anyway for copyright reasons. I have no idea why there is a G8, or, really, if it's even there. I thought that the issue with misdetection earlier was because I generated those sample files from a DAW that was running in WINE. I downloaded LMMS to try to generate a fresh file from a synth that was not going through wine, and now the tests fail again:
failures:
---- analyze::tests::test_get_notes_from_audio_file stdout ----
[Note { octave: Six, named_pitch: DFlat }, Note { octave: Five, named_pitch: A }, Note { octave: Five, named_pitch: GFlat }]
thread 'analyze::tests::test_get_notes_from_audio_file' panicked at 'assertion failed: notes.contains(&AFour)', src/analyze.rs:72:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- an
8000
alyze::tests::test_get_notes_from_mp3_file stdout ----
[Note { octave: Five, named_pitch: G }, Note { octave: Six, named_pitch: D }, Note { octave: Five, named_pitch: BFlat }]
thread 'analyze::tests::test_get_notes_from_mp3_file' panicked at 'assertion failed: notes.contains(&AFour)', src/analyze.rs:83:9
Here is the LMMS project in a zip file, perhaps you could do some testing to figure out why the analyzed notes aren't right? Does the test file sound like an Am chord compared to some instrument you have in tune?
I've also pushed my changes to this branch, including the tests that aren't working and the example files.
There was a problem hiding this comment.
Ok, let's keep what you have for now, and I will look into getting a better sample.
Can you address the other comments?
src/analyze.rs
Outdated
| let file = File::open(path)?; | ||
| let start = start.unwrap_or_default(); | ||
| let decoder = Decoder::new(file)?; | ||
| let decoder = decoder.skip_duration(start).convert_samples(); |
There was a problem hiding this comment.
Can you put this into one chained call?
| let decoder = Decoder::new(file)?; | ||
| let decoder = decoder.skip_duration(start).convert_samples(); | ||
| let sample_rate = decoder.sample_rate() as usize; | ||
| let samples: Vec<_> = if let Some(end) = end { decoder.take_duration(end - start).collect() } else { decoder.collect() }; |
There was a problem hiding this comment.
Go ahead and spell this out with carriage returns?
There was a problem hiding this comment.
I'm not sure what you mean. This is just following the formatting rules?
There was a problem hiding this comment.
Ahhh, damn. Didn't know the formatting would change that.
Ok, this is fine. Add some vertical white space to group operations?
| let decoder = Decoder::new(file)?; | ||
| let decoder = decoder.skip_duration(start).convert_samples(); | ||
| let sample_rate = decoder.sample_rate() as usize; | ||
| let samples: Vec<_> = if let Some(end) = end { decoder.take_duration(end - start).collect() } else { decoder.collect() }; |
There was a problem hiding this comment.
Ahhh, damn. Didn't know the formatting would change that.
Ok, this is fine. Add some vertical white space to group operations?
| let sample_rate = decoder.sample_rate() as f32; | ||
| let samples: Vec<_> = decoder.collect(); | ||
| let time = Duration::from_secs((samples.len() as f32 / sample_rate).ceil() as u64); | ||
| stream_handle.play_raw(SamplesBuffer::new(channels, sample_rate as u32, samples))?; |
There was a problem hiding this comment.
Same here. Can you add some vertical whitespace between that declarations and the function calls?
While reading the documentation for
rodioI foundsymphoniaand decided to redo from scratch using that rather than shelling out to FFMPEG.This implementation, however, still suffers from the same issue of inaccuracy as the previous one, leading me to think sample rate may still be an issue. Can you tell me what sample rate your test files are using?Edit: I tested this with a file with 48kHz sample rate and the result was the sameEdit 2: no, the problem is with the audio files themselves. My bad