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 110

Summary: SDL_mixer bugs
Product: SDL_mixer Reporter: Sam Lantinga <slouken>
Component: miscAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: blocker    
Priority: P2 CC: lordhavoc
Version: unspecified   
Hardware: All   
OS: All   
Attachments: SDL_mixer-1.2.6-effect_position.patch

Description Sam Lantinga 2006-01-30 00:41:43 UTC 
Date: Fri, 11 Mar 2005 07:57:35 +0100
From: David Ergo <david.ergo@alterface.com>
Subject: Re: [SDL] SDL_mixer bugs

Here is a patch to correct the 2 bugs and also other bugs.
Bug 1 (LFE channel saturated) has been corrected in a more generic way :
-    args->lfe_u8 = 255;
-    args->lfe_f = 255.0f;
+    args->lfe_u8 = speaker_amplitude[5];
+    args->lfe_f = ((float) speaker_amplitude[5]) / 255.0f;

Bug 2 has been corrected by adding code in _WIN32 "ifdef".

Other bugs corrected :
- in SetDistance(), SetPanning() and SetPosition(), if args where no-op
but effect was not already registered then it was registered instead of
doing a no-op.
- in SetPanning(), if left=right=255, it's a no-op so angle must be 0.
- there was some copy/paste errors in channel amplitudes (e.g. using
left instead of left_rear, etc.)
Comment 1 Sam Lantinga 2006-01-30 00:42:48 UTC 
Created attachment 53 [details]
SDL_mixer-1.2.6-effect_position.patch

Ryan, could you take a look at this patch, and apply it if it looks good?  Thanks!
Comment 2 Sam Lantinga 2006-01-30 00:44:36 UTC 
Here's a description of bug #2 that's addressed by this patch:

2. Sound channels interverted under Windows
-------------------------------------------
Windows DirectX doesn't use the same channels order as Linux ALSA.
For Linux ALSA, this is FL-FR-RL-RR-C-LFE
and for Windows DirectX, this is FL-FR-C-LFE-RL-RR
where FL = Front Left
      FR = Front Right
      RL = Rear Left
      RR = Rear Right
      C  = Center
      LFE = Low Frequency Effects
If you don't do any SetPosition()/SetPanning() there won't be any
problems I think because the channels data is not modified, but if you
do SetPosition() or SetPanning(), it will use C/LFE instead of RL/RR and
RL/RR instead of C/LFE, so the channels data will be messed up for
RL/RR/C/LFE.
Comment 3 Ryan C. Gordon 2006-01-30 11:23:01 UTC 
Ugh, we really need a way at the SDL level to guarantee that speakers are in a sane order...we shouldn't have to add #ifdefs to apps and libraries for each audio target that wants to use a different order!

--ryan.
Comment 4 Sam Lantinga 2006-01-31 09:10:56 UTC 
(In reply to comment #3)
> Ugh, we really need a way at the SDL level to guarantee that speakers are in a
> sane order...we shouldn't have to add #ifdefs to apps and libraries for each
> audio target that wants to use a different order!

I agree.  Should we add an API query or fix an order and remap the channels in the SDL audio driver?
Comment 5 Ryan C. Gordon 2006-01-31 10:46:25 UTC 
> I agree.  Should we add an API query or fix an order and remap the channels in
> the SDL audio driver?

It would make sense, in the same way that we let people request/force other parameters in SDL_OpenAudio(). Alternately, we can treat it like we treat stereo audio now: it's always in the same order (even if, say, the audio driver under the hood doesn't want the audio interleaved, this is hidden from the app).

I'd be inclined to say for 1.2, more-than-stereo audio has to be in a well-defined format. Since ALSA can bend more than DirectSound on this one, I'd say favor the Windows format. SDL should convert behind the scenes if necessary.

1.3 can add a means to get/set the format to avoid the conversion, probably as part of the AudioSpec handed to SDL_OpenAudio, which would let the app either avoid the conversion overhead or avoid caring about it, much like they currently do with frequency or channel conversion.

How's that sound?

--ryan.
Comment 6 Sam Lantinga 2006-01-31 12:53:47 UTC 
(In reply to comment #5)
> How's that sound?

That sounds great. :)  ALSA, take it in the pants! :)
Comment 7 Sam Lantinga 2006-05-01 06:53:25 UTC 
Ryan, I'd like to get this taken care of for SDL 1.2.10 and the next SDL_mixer release, if possible.  I assume that everything besides the Win32 ifdefs in the patch should be applied to SDL_mixer?

Also, what needs to happen inside SDL to switch to Windows channel order?
Comment 8 Sam Lantinga 2006-05-07 17:20:20 UTC 
I'd like to get this fixed for SDL 1.2.10 release, if possible.
Comment 9 Ryan C. Gordon 2006-06-21 03:19:11 UTC 
*** Bug 247 has been marked as a duplicate of this bug. ***
Comment 10 Sam Lantinga 2006-06-21 04:30:55 UTC 
It was mentioned in bug #247 that Mac OS X also uses the Windows channel ordering.
Comment 11 Ryan C. Gordon 2006-06-21 05:01:18 UTC 
That's makes it a no-brainer to make the change in ALSA, then. I need to read up on the API and see if you can change the order you feed it data, otherwise, we'll just have to swizzle the channels before feeding it through to the driver.

--ryan.
Comment 12 Sam Lantinga 2006-06-21 05:09:40 UTC 
(In reply to comment #11)
> That's makes it a no-brainer to make the change in ALSA, then. I need to read
> up on the API and see if you can change the order you feed it data, otherwise,
> we'll just have to swizzle the channels before feeding it through to the
> driver.
> --ryan.

Sounds good.  I bet ALSA has some option somewhere to control that...
Comment 13 Ryan C. Gordon 2006-06-23 04:35:04 UTC 
Comment on attachment 53 [details]
SDL_mixer-1.2.6-effect_position.patch

Not patching SDL_mixer for this bug, but SDL instead, so I'm obsoleting this patch.

--ryan.
Comment 14 Ryan C. Gordon 2006-06-23 04:36:09 UTC 
Potential fix is now in SDL (not SDL_mixer!) Subversion. I don't have a 5.1 setup to test it, so please try it and reopen this bug if I botched it.

--ryan.
Comment 15 Sam Lantinga 2006-06-23 04:51:38 UTC 
Ryan, it looks like there were a couple other fixes in that patch, besides the 5.1 audio swizzle.  Can you check to make sure they aren't needed?
Comment 16 Ryan C. Gordon 2006-06-23 05:08:16 UTC 
Took the win32 bits out of the patch, committed the rest.

--ryan.