| Summary: | [Patch] Fixed inter-chunk clicks because inaccuracy of the length calculation while resampling | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Vitaly Novichkov <admin> |
| Component: | audio | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | major | ||
| Priority: | P2 | CC: | admin, slouken |
| Version: | HG 2.1 | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: |
More accurate resampling (outdated)
More accurate resampling (Fixed comments) More accurate resampling (Up to 838b7f845f84 revision) Result of resampler with enabled attempt of interpolation Comparison of resampler implementations (8124 to 44100 hz) Comparison of resampler implementations (95498 and 96000 to 44100 hz) Experimental SDL_audiotypecvt.c file (contains both current and my versions) More accurate resampling (Up to 2a0d717d55be revision) with disabled interpolator More accurate resampling (Up to 2a0d717d55be revision) with kept interpolator |
||
Created attachment 2655 [details]
More accurate resampling (Fixed comments)
Oops, I accidentally used non-ANSI comments (I see that SDL is targeted to be built by old compilers too, therefore use only multiline comments)
Created attachment 2656 [details]
More accurate resampling (Up to 838b7f845f84 revision)
More accurate lengths calculation while resampling (up to 838b7f845f84 revision)
*** Bug 58 has been marked as a duplicate of this bug. *** Created attachment 2663 [details]
Result of resampler with enabled attempt of interpolation
The result of "interpolation" attempt. Without interpolation, those clicks are not presenting. Therefore I added a macro to toggle it off. This patch is outdated and is not compatible with the latest revision. Now I'm working to update patch. Current resampler still to produce a sounds with clicks.
Created attachment 2664 [details]
Comparison of resampler implementations (8124 to 44100 hz)
Here is an 8-bit stereo sound of 8124 Hz (I generated a sound with the non-regular frequency for better testing) and results of resampling it into 44100 Hz.
- Sine_8124-to-44100_8_2-before.wav - resampled by current revision fe9c7d01a093 with no changes
- Sine_8124-to-44100_8_2-after.wav - resampled by my fixed version with interpolation
- Sine_8124-to-44100_8_2-nointerpolate.wav - resampled by my fixed version with disabled interpolation (just do copy same sample)
Comment on attachment 2656 [details]
More accurate resampling (Up to 838b7f845f84 revision)
No more compatible with latest revision
Created attachment 2665 [details]
Comparison of resampler implementations (95498 and 96000 to 44100 hz)
Added down-sample comparison with and without my fix
Created attachment 2666 [details]
Experimental SDL_audiotypecvt.c file (contains both current and my versions)
In this file are kept both old and new versions which are can be toggled by macro:
USE_OLD_RESAMPLER_CODE - to build with old resampling code (unmodified). Without macro new version will be built
TRY_INTERPOLATE_ARBITRARY - Enable pseudo-interpolation code (which in real doesn't work, because just causes offset of some samples, but also producing the inter-chunk clicks, which are shown in the picture also included here)
Anyway, the solution to avoid those clicks is store "last sample" in the "SDL_AudioCVT" structure and re-use it every next resample call. What are you think about that? Created attachment 2667 [details]
More accurate resampling (Up to 2a0d717d55be revision) with disabled interpolator
Updated patch which fixes inaccurate length calculation and resolves the inter-chunk clicks
Created attachment 2668 [details]
More accurate resampling (Up to 2a0d717d55be revision) with kept interpolator
Updated patch which fixes inaccurate length calculation and resolves the inter-chunk clicks. There are still be in resampled sound, because of incorrect linear interpolation implementation. (which suggested to be disabled or be improved)
Hey, thank you for all your attention and effort on this, but I decided to change out the resampler with something that's faster and simpler but still produces acceptable quality output: https://hg.libsdl.org/SDL/file/1502bb751ca4/src/audio/SDL_audiocvt.c#l194 Your fixes here were good, but it felt like replacing this code, rather than fixing it, was the right action to take. --ryan. (In reply to Ryan C. Gordon from comment #12) > Hey, thank you for all your attention and effort on this, but I decided to > change out the resampler with something that's faster and simpler but still > produces acceptable quality output: > > https://hg.libsdl.org/SDL/file/1502bb751ca4/src/audio/SDL_audiocvt.c#l194 > > Your fixes here were good, but it felt like replacing this code, rather than > fixing it, was the right action to take. > > --ryan. Sorry, I missed to read your reply here, therefore I found that you are replaced resample before reading this reply. I got new code and reviewed resampler myself. But it's much more buggy than was before, so, I made another issue with all my researches. I'll try to fix it myself if I will get a time tomorrow (moment I wrote this is 1:45am on my timexone), if you will not fix it before. It's very important to have fine resampler, because users of my game engine and editor are using random music (most case is 16-bit 44100) and there are had strong troubles with bad resampler, therefore I suggested them resample to 44100, but some of users are so stupid [facepalm], and I annoyed to explain them this thing, even in case I have gas... Libresample seems good thing, but I still want to use a simple resampler. |
Created attachment 2653 [details] More accurate resampling (outdated) I took care and finally fixed clicks between chunks while resampling. A Reason of that was the inaccurate calculation of the lengths. That eps was weird and seems worked inaccurate in some moments, but using double and iterating by the pieces of ratio produces a more accurate sample iteration. P.S. I also added an ability to toggle off the attempt to interpolate by macro, because in some moments result a bit glitchy (clicks and clips while resampling in both directions). I kept it be always defined, but you can move it into another and more correct place. Patch made based on latest revision f6cd81aab88e on the moment of posting this.