<br><br><div class="gmail_quote">On Mon, Mar 28, 2011 at 7:15 PM, Max Horn <span dir="ltr"><<a href="mailto:max@quendi.de">max@quendi.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi folks,<br>
<br>
as you may have noticed, recently I added a new class Audio::MidiPlayer and converted several engines to use this new class, specifically<br>
  agi, draci, hugo, m4, made, parallaction, saga, tinsel, touche.<br>
Others could possibly follow, e.g. lure, groovie, queen, maybe even agos.<br></blockquote><div><br></div><div>SCI should follow too, by the looks of it </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Here are some examples of differences. I hope most of them can be removed eventually.<br>
<br>
* The default value of the _channelsVolume[] array:<br>
In Audio::MidiPlayer, I use the default value 127 for all channels. Some engines used 0 instead. And lure uses 90, saying "90 is power on default for midi compliant devices"<br>
Actually, it is not even clear to me whether this default value matters at all, but finding a common default value (with justification for it ;) seems desirable.<br></blockquote><div><br></div><div>I suppose that a standard median starting value should make sense to make as default for all engines here.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
* The way MidiDriver::detectDevice is used:<br>
  Most engines use "MDT_MIDI | MDT_ADLIB | MDT_PREFER_GM" as flags, but e.g. AGI just uses "MDT_MIDI | MDT_ADLIB". It is in general not evident why one value or another is used. We should unify this, or at least document the difference.<br>
</blockquote><div><br></div><div>That's because AGI games did not support GM, so that difference is to be expected </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

* In most engines, the setVolume method does nothing if the new volume matches the old volume; in some engines, it always does something. This could lead to different behavior in certain cases.<br></blockquote><div><br></div>
<div>Actually, it would be good if there was a standard setVolume method, which could be overriden by each engine in case of specialized values.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

* A few engines (agi, draci, hugo) handle newly created MidiChannel's differently, taking extra care to call the volume() method of the new channel, thus enforcing the aforementioned default channel volume. Is this really necessary? If yes, then should we introduced it for other engines, too? Otherwise, explain / document the difference.<br>
</blockquote><div><br></div><div>I noticed that one too and found it odd myself :/</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
* Handling pause / resume varies a lot: The current (arbitrary) default implementation for pause() to set _isPlaying to false, and then set the volume to 0 (actually, -1, which is clipped to 0... weird). Resume is done by setting _isPlaying back to true, and setting the volume to match the music volume specified in ConfMan.<br>

<br>
One issue with that is that while paused, a MidiPlayer instance returns potentially "confusing" information about itself; e.g. isPlaying() returns false (which can be argued to be right or wrong, depending on what one things isPlaying() should mean), and also getVolume() returns 0.<br>

<br>
Some engines implement pause/resume by introducing a dedicated _isPaused boolean. And these generally also act slightly different on resume(), by setting the volume to whatever it was before the pause.<br>
This may sometimes be "better" (if a game is temporarily modifying the music volume, independently from user settings), or worse (if the music was paused in order to let the user tweak the music volume, then upon resume, we want the midi player to use the new volume!).<br>

<br>
Hence it is not necessarily clear which behavior is "correct"; in fact, this is likely to depend on the rest of the engine's code. Still, it would seem to me that we should pick one way, document and implement that consistently, and adapt the engines to it.<br>
</blockquote><div><br></div><div>Agreed, this should be a bit standardized...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
* Some engines explicitly detect and handle MT-32 type devices, some totally ignore this possibility. Of course not all engines have games which makes use of an MT-32. I wonder if these then should explicitly error out when an MT-32 type device is detected? Or use some kind of converter? In any case, it seems dangerous to just ignore the possibility.<br>
</blockquote><div> </div><div>Perhaps we could fall back to Adlib in these cases?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
* Most engines make use of the sendMT32Reset / sendGMReset methods, but not all.<br></blockquote><div><br></div><div>Is this the responsibility of the engines? Shouldn't this be handled at the MIDI parser level?</div>
<div><br></div><div>Regards</div><div>Filippos</div></div><br clear="all"><br>-- <br>"Experience is the name every one gives to their mistakes" - Oscar Wilde <br>