SampleRateConverter converts frames at wrong rate

[TonalidadeHidrica]

When I played a 48000Hz mono-channel ogg file converted by UniformSourceIterator, in which the sample rate is converted in SampleRateConverter, the conversion rate seems to be slightly wrong.

First I generated a 48000Hz mono-channel ogg file which includes a metronome sound at 120 BPM for 400 beats with Audacity. Then I played it by this library, output the sound from the speaker, where the device output sample rate is set to 44100Hz, and record the sound from the speaker with the microphone via Audacity again. Then, the duration between the 0th and 200th beat was exactly 100.050 seconds. When I synthesize the wave derived from the ogg file with another tone constantly made 22050 samples, the 0th and 200th beat has the exact elapsed time of 100.000 seconds. The gap between the two source was becoming proportionally larger.

When I did the same experiment with 44100- and 22050-Hz ogg file, this gap didn't happen. We can infer that there is something wrong with conversion of sample rate. I have not made an in-depth inspection on the code though.

Since I found several other issues in SampleRateConverter other than this, I didn't fix the converter. That's why I'm just submitting an issue. FWIW, this is an alternative implementation I wrote. In this converter, I used floating-point operations to make a linear interpolation, so it may not be as concise as the converter in this library. Nevertheless my code does not have the issue of conversion error.

[Rua]

This might have the same cause as #303. What happens if you use rustup override to force it to use Rust 1.44.1?

[TonalidadeHidrica]

In short, the issue exists as well in 1.44.1.

I created a minimal example to reproduce the issue. You can see the code in my fork. I wanted to attach the ogg files I used, but OGG files are not allowed for GitHub Issue attachments (how can I?).

The following is the results:

[N] ⋊> ~/d/g/c/g/T/rodio on sandbox/issue-316  rustup run 1.46.0 cargo run --example issue_316 ~/Documents/Metronome2/M2_44100.ogg                                                                                                  17:19:03
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `target/debug/examples/issue_316 /Users/username/Documents/Metronome2/M2_44100.ogg`
Original sample rate = 44100, Output sample rate = 44100
  1 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  2 -------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^-vvvvvvvvv-
  3 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  4 --------------------------------------------------------------------------------------------------------^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  5 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvv-
  6 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvv-
  7 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  8 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  9 --------------------------------------------------------------------------------------------------------^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv^^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvv-
 10 --------------------------------------------------------------------------------------------------------^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv
[I] ⋊> ~/d/g/c/g/T/rodio on sandbox/issue-316  rustup run 1.46.0 cargo run --example issue_316 ~/Documents/Metronome2/M2_48000.ogg                                                                                                  17:19:32
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/examples/issue_316 /Users/username/Documents/Metronome2/M2_48000.ogg`
Original sample rate = 48000, Output sample rate = 44100
  1 --------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^
  2 -------------------------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^
  3 ------------------------------------------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vv
  4 -----------------------------------------------------------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvv
  5 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvv
  6 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  7 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  8 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  9 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 10 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
[I] ⋊> ~/d/g/c/g/T/rodio on sandbox/issue-316  rustup run 1.44.1 cargo run --example issue_316 ~/Documents/Metronome2/M2_44100.ogg                                                                                                  17:19:37
   Compiling rodio v0.11.0 (/Users/username/dev/git/com/github/TonalidadeHidrica/rodio)
    Finished dev [unoptimized + debuginfo] target(s) in 2.48s
     Running `target/debug/examples/issue_316 /Users/username/Documents/Metronome2/M2_44100.ogg`
Original sample rate = 44100, Output sample rate = 44100
  1 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  2 -------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^-vvvvvvvvv-
  3 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  4 --------------------------------------------------------------------------------------------------------^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  5 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvv-
  6 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvv-
  7 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  8 -------------------------------------------------------------------------------------------------------^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv^^^^^^^^^^vvvvvvvvvv
  9 --------------------------------------------------------------------------------------------------------^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv^^^^^^^^^^vvvvvvvvvv^^^^^^^^^^vvvvvvvvv-
 10 --------------------------------------------------------------------------------------------------------^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv
[I] ⋊> ~/d/g/c/g/T/rodio on sandbox/issue-316  rustup run 1.44.1 cargo run --example issue_316 ~/Documents/Metronome2/M2_48000.ogg                                                                                                  17:19:44
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `target/debug/examples/issue_316 /Users/username/Documents/Metronome2/M2_48000.ogg`
Original sample rate = 48000, Output sample rate = 44100
  1 --------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^vvvvvvvvvv^^^^^^^
  2 -------------------------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^
  3 ------------------------------------------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvvvvvv-^^^^^^^^^-vv
  4 -----------------------------------------------------------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvvv-^^^^^^^^^-vvvvv
  5 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------^^^^^^--vvvvvvvv
  6 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  7 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  8 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  9 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 10 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

The line numbered i indicates the waveform around the i-th second. Theoretically each waveform should be aligned vertically, but it isn't when converted from 48000Hz to 44100Hz.

By the way, if I created a equivalent WAV file (instead of OGG) and used it, the issue did not occur. Maybe it's not just about the SampleConverter.

[nbraud]

FYI, I wrote a reproducing testcase, in preparation for attempting to refactor the resampler. :)

[nbraud]

@TonalidadeHidrica I ended up sending a PR (#324) with a couple of bugfixes and a testcase for this issue, but no fix for it quite yet: I don't think I can reasonably fix this without first refactoring SampleRateConverter, which is proving not to be a simple job >_>'

Anyhow, I hope I can have a fix sometime next week :)

[TonalidadeHidrica]

@nbraud Thanks for your effort. The SampleConverter is indeed complicated, and IMHO, it's totally unnecessary complexity. After a couple of hours of reading, I found out that it can be written much simpler...

[nbraud]

@TonalidadeHidrica Yeah, I'm pretty sure it ought to be simplified a lot; what it's actually doing is just finding the 2 input samples in-between which the (current) output sample falls, and interpolating linearly between the two.

What I was planning to do was first refactor and simplify the current implementation, fix this issue, and then look at better resampling algorithms to implement (windowed sinc interpolation might be a big win already, but TBH it might make more sense to go straight for band-limited interpolation)

[est31]

Fixed by #324

[est31]

Oh wait, author says it wasn't fixed. My bad.