Re: [AD] Latest mixer patch

[Elias Pschernig <elias@xxxxxxxxxx>]

On Mon, 2004-07-05 at 19:05 -0700, Chris wrote:
> This is the latest patch I've thrown together. I don't quite remember 
> what the last patch contained, so I'll just do a recap of this one.
> 

Thanks, that's very helpful.

> * Full 24-bit mixing instead of 14-bit.
> * The interpolation mixer uses the full 24-bit range for all sample 
> depths, instead of 8-bit interpolation for 8-bit samples and 16 for 16-bit.
> * set_volume_per_voice (which is back to the regular behavior) can be 
> called any time, not just before install_sound.
> * Increased internal volume precision.
> * Removed the clipping table, since 24-bit mixing on a 32-bit buffer 
> will be impossible to overflow with Allegro's voice max.
> * Uses less memory.

And according to the comments, also fixes a possible memory leak if
mutex creation fails in _mixer_init :)

> 
> This patch does /not/ include the mixer stream stuff. The reason being 
> the mixer streams won't work right with the DX mixer and it would only 
> add to the time it takes to get 4.2 out (which has already increased 
> enough). It does however add these functions:
> set_mixer_quality
> get_mixer_quality
> get_mixer_frequency
> get_mixer_depth
> get_mixer_voices
> get_mixer_channels
> get_mixer_buffer_length
> They are documented in the source so don't worry about that. However, 
> they still suffer the same problem as the mixer streams: they won't work 
>   with the DX mixer active. Hopefully though, the AllegMix driver can be 
> made the default and if you do try to use them with the DX mixer driver 
> they'll just return 0, so you can still compensate.
> 

I just tested it, and the set_volume_per_voice works now like we agreed
on in the last discussion of this a while back - set_volume_per_voice(1)
is the new default volume (so Allegro apps are cured of the low default
volume problem, which was probably more a problem than clipping,
especially with thw new mixer who can only clip and not wrap). I'll add
a comment about this to the docs and ABI compatibility section when
committing.

I read through your changes (without trying to understand them :P) - and
only made some minor formatting changes, and I used ASSERT() instead of
modifying user provided parameters for set_mixer_quality and
_set_mixer_position - since that's the way Allegro normally treats user
parameters. (So instead of silently allowing errors to creep into the
user code, the program aborts.)

+   if((quality < 0) || (quality > 2))
+      quality = 2;

+   ASSERT(_sound_hq >= 0 && _sound_hq <= 2);

and

+   if (position < 0)
+      position = 0;

+   ASSERT(position >= 0);

Else it looks ok for me to finally commit it. I'll wait until the
weekend for complaints against that - but from what I remember Ben, Bob
and Thomas already agreed to using your mixer, with the proper
set_volume_per_voice.

-- 
Elias Pschernig