We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3551 - [Patch] Fixed bad resampling recently implemented in the SDL_ResampleAudioSimple (revision 938218064f67)
Summary: [Patch] Fixed bad resampling recently implemented in the SDL_ResampleAudioSim...
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: audio (show other bugs)
Version: HG 2.1
Hardware: All All
: P2 major
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-09 21:45 UTC by Vitaly Novichkov
Modified: 2017-01-18 07:13 UTC (History)
1 user (show)

See Also:

Attachments
A script to generate a testing files from one (it will generate various WAV files of various sample sizes and rates) (407 bytes, application/x-sh)
2017-01-09 21:45 UTC, Vitaly Novichkov 		Details
Demo of bug (source and recorded result) (2.52 MB, application/x-7z-compressed)
2017-01-09 21:48 UTC, Vitaly Novichkov 		Details
Experimental SDL_audiocvt.c whith my attempt to implement a fix (33.50 KB, text/x-csrc)
2017-01-10 23:24 UTC, Vitaly Novichkov 		Details
Fixed resampler (1.40 KB, patch)
2017-01-11 10:31 UTC, Vitaly Novichkov 		Details | Diff
Fixed resampler (1.40 KB, patch)
2017-01-15 16:34 UTC, Vitaly Novichkov 		Details | Diff
Fixed resampler (1.40 KB, patch)
2017-01-15 17:00 UTC, Vitaly Novichkov 		Details | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Vitaly Novichkov 2017-01-09 21:45:58 UTC 
Created attachment 2670 [details]
A script to generate a testing files from one (it will generate various WAV files of various sample sizes and rates)

Just got review new changes in SDL Audio, and I though you are finally implemented right resampler and no more need to patch it, but a bug appears again...

Just my small gift for you - I made a small script which I used to quickly generate various WAV files (by using SoX utility) of various frequencies and bit sizes to test the resampling better. Use it (or any similar thing).

A Bug is same on both ways (up-sample and down-sample) and appears with 8-bit source samples. With 16-bit samples is fine (finally!).

Today I have no time to work, I hope you can fix this bug, or I'll try to fix it myself.

Some research:
- I tried to dump U8-to-F32, result seems fine.
- I tried to dump F32-to-U16, result is broken, but chain is going after SDL_ResampleAudioSimple function. Is very weird that this happen with 8-bit sound, mayby you forgot to update some length and in result a broken result?
Comment 1 Vitaly Novichkov 2017-01-09 21:48:01 UTC 
Created attachment 2671 [details]
Demo of bug (source and recorded result)

This is my snapshot of buggy result. Source file (Unsigned 8-bit, 11025 hz Stereo), and resampled result (Signed 16-bit, 44100 hz Stereo)
Comment 2 Vitaly Novichkov 2017-01-09 21:52:58 UTC 
EDIT: Bug is reproducing on ANY sound output, no matter bits. "Fine" result I got because in my player still be used my custom side resampler (which working for the 16-bit sound only) which now I disabled and just found that bug is no matter to source sample size.
Comment 3 Vitaly Novichkov 2017-01-10 23:24:48 UTC 
Created attachment 2672 [details]
Experimental SDL_audiocvt.c whith my attempt to implement a fix

I have to implement a working resampler which is based on the old code of arbitrary resampler, but with the (rate_incr < 1.0) condition to choose to upsample or downsample, but I still need to fix an invalid code to have one "while" loop for both directions.
Comment 4 Vitaly Novichkov 2017-01-11 10:31:02 UTC 
Created attachment 2674 [details]
Fixed resampler

I finally implemented the working two-direction resampler. Also, I made a small optimization and "consumed" variable is not needed.
Comment 5 Vitaly Novichkov 2017-01-11 20:18:11 UTC 
P.S. Why you choose the "libsamplerate" instead of the "libsoxr"?
Audacity team are choose the libsoxr for their purposes: http://wiki.audacityteam.org/wiki/Libsoxr
Comment 6 Vitaly Novichkov 2017-01-15 16:34:02 UTC 
Created attachment 2676 [details]
Fixed resampler
Comment 7 Vitaly Novichkov 2017-01-15 16:38:21 UTC 
Just been updated patch to fit into recent change (adding two spaes)
Comment 8 Vitaly Novichkov 2017-01-15 16:59:59 UTC 
Comment on attachment 2676 [details]
Fixed resampler

Ouch, typo (forgot to add some space, and result is rejecting patch)
Comment 9 Vitaly Novichkov 2017-01-15 17:00:41 UTC 
Created attachment 2677 [details]
Fixed resampler

Now it's finally compatible with latest 938218064f67 revision
Comment 10 Ryan C. Gordon 2017-01-17 19:27:07 UTC 
(In reply to Vitaly Novichkov from comment #9)
> Created attachment 2677 [details]
> Fixed resampler
> 
> Now it's finally compatible with latest 938218064f67 revision

This patch looks reasonable; I'll try it today and hopefully apply it!

--ryan.
Comment 11 Ryan C. Gordon 2017-01-18 07:13:39 UTC 
(In reply to Ryan C. Gordon from comment #10)
> (In reply to Vitaly Novichkov from comment #9)
> > Created attachment 2677 [details]
> > Fixed resampler
> > 
> > Now it's finally compatible with latest 938218064f67 revision
> 
> This patch looks reasonable; I'll try it today and hopefully apply it!
> 
> --ryan.


Okay, your patch is now https://hg.libsdl.org/SDL/rev/efc103e60c5b

Thanks!

--ryan.