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 4878 - [PATCH] Extended WAV support
Summary: [PATCH] Extended WAV support
Status: RESOLVED FIXED
Alias: None
Product: SDL_mixer
Classification: Unclassified
Component: misc (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal
Assignee: Ozkan Sezer
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-27 12:54 UTC by Vitaly Novichkov
Modified: 2020-10-25 21:12 UTC (History)
2 users (show)

See Also:

Attachments
Extended support for WAV and AIFF (23.21 KB, patch)
2019-11-27 12:55 UTC, Vitaly Novichkov 		Details | Diff
Example uLAW-encoded WAV file with 8000 hz (945.42 KB, audio/wav)
2019-11-27 13:15 UTC, Vitaly Novichkov 		Details
A set of wave sampls of different sample formats (5.28 MB, application/x-7z-compressed)
2019-11-27 13:22 UTC, Vitaly Novichkov 		Details
Extended support for WAV and AIFF (26.02 KB, patch)
2019-12-03 09:38 UTC, Vitaly Novichkov 		Details | Diff
Show Obsolete (1) 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 2019-11-27 12:54:58 UTC 
On MixerX fork I made a big extending of WAV parser where I gave next features:

- Support Float32, Float64 both big and little-endian, all variants
  of integer (8, 16, 24 and 32) both little and big-endian as a sample format
- Better support of AIFF files
- Support for uLAW and ALAW encodings (there are small functions from an MIT licensed code)
- Added WAV_Seek() call to allow seeking inside of WAV or AIFF files

https://bitbucket.org/Wohlstand/sdl_mixer/commits/6585d9dfbb9f5dff0dfba0a86ae3debec254ac82

Note: you'll find uLAW and ALAW decoding pieces are MIT-licensed. I think, they shouldn't conflict with ZLib licenses as I did a small comparison of both licenses, and they are compatible (MIT is even more permissive than ZLib which additionally tells to don't steal authorship and don't present forks and mods as originals). You can give your comments on this thing or replace uLAW and ALAW code with that you think is better (probably SDL2 itself has it? Maybe copy it to here rather use this?)
Comment 1 Vitaly Novichkov 2019-11-27 12:55:55 UTC 
Created attachment 4073 [details]
Extended support for WAV and AIFF
Comment 2 Vitaly Novichkov 2019-11-27 13:15:46 UTC 
Created attachment 4074 [details]
Example uLAW-encoded WAV file with 8000 hz
Comment 3 Ozkan Sezer 2019-11-27 13:18:57 UTC 
Didn't review/test the patch, however one should be aware
that the borrowed g711.c code has a different license than
zlib: https://github.com/escrichov/G711/blob/master/g711.c
IANAL, so a matter for the ones who know about such things.
Comment 4 Vitaly Novichkov 2019-11-27 13:22:56 UTC 
Created attachment 4075 [details]
A set of wave sampls of different sample formats

A set of test wave files of different formats. Many of them won't play on SDL2_mixer, however, they are playable with this patch.
Comment 5 Vitaly Novichkov 2019-11-27 13:28:09 UTC 
ALSO! I totally forget that some of my wave samples have stuck the SDL2_mixer previously (mainly on an attempt to open one of unsupported sample formats). My patch fixes also this bug.
Comment 6 Ozkan Sezer 2019-11-27 13:38:33 UTC 
(In reply to Vitaly Novichkov from comment #5)
> ALSO! I totally forget that some of my wave samples have stuck the
> SDL2_mixer previously (mainly on an attempt to open one of unsupported
> sample formats). My patch fixes also this bug.

Can you point to the exact part(s) of the patch which fixes
that issue?
Comment 7 Vitaly Novichkov 2019-11-27 14:56:37 UTC 
(In reply to Ozkan Sezer from comment #6)
> Can you point to the exact part(s) of the patch which fixes
> that issue?

It's a global problem that can't be just quickly fixed. I did the major rework of the parse which together with avoiding a stuck, adds support for formats is previously didn't supported. I did a check, and there are s8 AIFF files making a stuck, u8 won't work, other AIFF files also won't work.

BTW, I have found a commit on MixerX side which explains the stuck fix with AIFF files and the reason if them: https://github.com/WohlSoft/SDL-Mixer-X/commit/d3dfe0a8abb400e1913d6125cea49ac0adf42521
Comment 8 Ozkan Sezer 2019-11-27 15:00:11 UTC 
I see.

(Is it me, or does this bugzilla not send me email even if I'm
on the CC list?)
Comment 9 Vitaly Novichkov 2019-11-27 15:03:29 UTC 
(In reply to Ozkan Sezer from comment #8)
> I see.
> 
> (Is it me, or does this bugzilla not send me email even if I'm
> on the CC list?)

We are both in CC list, I also didn't receive any emails of this...
Comment 10 Ozkan Sezer 2019-11-27 15:13:03 UTC 
(In reply to Vitaly Novichkov from comment #9)
> (In reply to Ozkan Sezer from comment #8)
> > (Is it me, or does this bugzilla not send me email even if I'm
> > on the CC list?)
> 
> We are both in CC list, I also didn't receive any emails of this...

Not just this one: I haven't been receiving emails for other bugs
which I'm on CC list.  (Possibly you, too.)
Comment 11 Vitaly Novichkov 2019-12-03 09:38:36 UTC 
Created attachment 4087 [details]
Extended support for WAV and AIFF

Just now I have updated the code: I have replaced alien code with SDL_wave.c's one. Should be okay now.
Comment 12 Sam Lantinga 2019-12-04 06:37:25 UTC 
This seems good to me. Ozkan, can you coordinate this with the other changes going into SDL_mixer?
Comment 13 Ozkan Sezer 2019-12-04 15:50:25 UTC 
- Edited the commit message a bit, removed the stale MIT-license notice
- Fixed warning by removing unused local var chunk_buffer:
  music_wav.c: In function 'LoadAIFFMusic':
  music_wav.c:891: warning: unused variable 'chunk_buffer'
- Fixed build failure in ALAW_To_PCM16() by replacing u_val with a_val
  when SDL_WAVE_LAW_LUT is defined (please confirm.)

Pushed as: https://hg.libsdl.org/SDL_mixer/rev/ae77864d2fa7

Question: Do we want to define SDL_WAVE_LAW_LUT or not?
Comment 14 Ozkan Sezer 2019-12-04 19:47:54 UTC 
> Question: Do we want to define SDL_WAVE_LAW_LUT or not?

Sam, Ryan: We don't define SDL_WAVE_LAW_LUT in SDL2/SDL_wave.c,
either. Is there a reason? It should be the fastest path, no?
Comment 15 Ozkan Sezer 2019-12-17 08:04:53 UTC 
(In reply to Ozkan Sezer from comment #14)
> > Question: Do we want to define SDL_WAVE_LAW_LUT or not?
> 
> Sam, Ryan: We don't define SDL_WAVE_LAW_LUT in SDL2/SDL_wave.c,
> either. Is there a reason? It should be the fastest path, no?

PING?  The question is for both SDL and for SDL_mixer.
Comment 16 Ozkan Sezer 2020-10-25 21:12:07 UTC 
This can be closed (SDL_WAVE_LAW_LUT definition is not that relevant.)