SV: SV: [Request for review and bugid] A collection of fixes for gervill (2010-04)
Alex Menkov
alex.menkov at sun.com
Thu Apr 8 03:40:05 PDT 2010
Hi Karl,
Thank you, it looks fine.
Feel free to push the fix.
About Preferences: I understand the reason, but it's unclear for me how
it should be used (by developers or users).
Am I right that it supposes creation of a "config utility" which allows
to set synthesizer properties and saves them into Preferences storage
(and this utility should be run by end user to configure default
properties)?
regards
Alex
Karl Helgason wrote:
> Hi,
> I have made those changes you talked about:
> http://cr.openjdk.java.net/~kalli/gervill-update/webrev.03/
>
> I'm however not sure if I got all code formatting correct.
>
>> About using Preferences (support for user config file).
>> It's not evident for me that the synthesizer needs the feature. Could
>> you please describe how it's supposed to be used?
>
> Only way now to change synthesizer settings is to use the
> AudioSynthesizer interface. But the problem is that
> most programs don't know about it.
> So the only way to change those settings would be
> to allow user to change the default settings
> using for example the Preference API.
>
> One reason for changing the default settings would
> be that users want to use different audio format
> or require lower latency settings, it is by default 120 msec
> but on windows we can go as low as 20 msec latency
> and and other platforms we can go even lower.
>
> regards,
> Karl
> ________________________________________
> Frá: Alexey.Menkov at Sun.COM [Alexey.Menkov at Sun.COM] Fyrir hönd Alex Menkov [alex.menkov at Sun.COM]
> Sent: 7. apríl 2010 16:30
> Viðtakandi: Karl Helgason
> Afrit: jdk6-dev at openjdk.java.net
> Efni: Re: SV: [Request for review and bugid] A collection of fixes for gervill (2010-04)
>
> Hi Karl,
>
> my comments about the fix:
>
> src/share/classes/com/sun/media/sound/AudioFloatFormatConverter.java
> could you please remove empty statements at lines 178 & 188 (the lines
> consist of only ";" symbol)
>
>
> src/share/classes/com/sun/media/sound/SoftChannel.java
> year in the copyright header has been changed from 2007 to 2010
> I believe 2007-2010 would be more correct.
>
>
> src/share/classes/com/sun/media/sound/SoftSynthesizer.java
>
> line 28: +import java.awt.image.BufferedImage;
> the import is definitely unnecessary.
>
> getDefaultSoundbank method:
> Now you search for available soundbank (jre home, windows system
> soundbank, saved soundbank) and open InputStream (returned from
> doPrivileged). Then use MidiSystem.getSoundbank() to get soundbank from
> the stream.
> If, for example, jre home have a soundbank in lib/audio, but the file is
> corrupted, MidiSystem.getSoundbank() throws exception and windows system
> sounbank & saved soundbank won't be tried to load.
> I think old algorithm was better (in the case I described it tries to
> load soundbank from other locations).
> (Note that you are completely right that MidiSystem.getSoundbank() must
> be called outside of privileged code to prevent security vulnerability)
>
> About using Preferences (support for user config file).
> It's not evident for me that the synthesizer needs the feature. Could
> you please describe how it's supposed to be used?
> And couple notes about the implementation:
> - Could you reformat the code like the rest of the code (now it looks
> like code in "C/C++ style")
> - AudioFormat parsing (lines 966-995): StringTokenizer uses default
> delimiters. I think it would be better to add comma to the delimiter
> list - it would add compatibility with AudioFormat.toString
> implementation (it uses commas). for the same reason "channels" would be
> better than "ch".
>
>
> regards
> Alex
>
> Karl Helgason wrote:
>> Hi,
>>
>> I added the missing "newline at the end of the file" to both SkipTest.java and ModelStandardIndexedDirector.java.
>>
>> http://cr.openjdk.java.net/~kalli/gervill-update/webrev.02/
>>
>> regards,
>> Karl
>> ________________________________________
>> Frá: joe.darcy at oracle.com [joe.darcy at oracle.com]
>> Sent: 6. apríl 2010 02:20
>> Viðtakandi: Karl Helgason
>> Afrit: jdk6-dev at openjdk.java.net; alexey.menkov at sun.com; joe.darcy at sun.com; Dalibor.Topic at Sun.COM
>> Efni: Re: [Request for review and bugid] A collection of fixes for gervill (2010-04)
>>
>> Hello.
>>
>> I've filed bug 6941027 "Gervill update, April 2010" for this work.
>> However, I will leave Alexey to do the actual code reviewing for OpenJDK
>> 6 and JDK 7.
>>
>> Note that
>> test/javax/sound/midi/Gervill/AudioFloatFormatConverter/SkipTest.java
>> doesn't have a newline at the end of the file.
>>
>> -Joe
>>
>> On 4/5/2010 1:29 PM, Karl Helgason wrote:
>>> Hi,
>>>
>>> I need code review and bugid for the fix:
>>> http://cr.openjdk.java.net/~kalli/gervill-update/webrev.01/
>>>
>>> Major fixes this webrev includes are:
>>>
>>> * Allow access to cached emergency soundbank using AccessController.doPrivileged
>>> This fix allows using cached emergency soundbank in applets
>>> which speeds up loading of applets especially on slow computers.
>>> And also using AccessController.doPrivileged we can use user-installed
>>> soundfonts in applets e.g. soundfonts found in audio folder of JRE.
>>>
>>> * Support for user config file.
>>> Default settings for Gervill don't always suit everybody,
>>> some needed lower latency, different samplerate and e.t.c
>>> To fix that a support to allow user to change default settings using
>>> Preferences API was added.
>>>
>>> * Fixed how getAvailableInstruments and getLoadedInstruments worked.
>>> SoftSynthesizer.getAvailableInstruments now always returns instruments
>>> from default soundbank. And getLoadedInstruments now always returns
>>> loaded instruments. The behavior of these methods where not correct.
>>>
>>> * Improved support for large soundbanks
>>> a) Indexed version of ModelStandardDirector added which speeds up
>>> looking up notes in instruments with many layers.
>>> b) Allow unused instrument to be garbage collected more easily,
>>> we now put null value to unused variable.
>>> c) Prevent unnecessary lookup of instrument object
>>> on pseudo program change (when bank and program is unchanged)
>>>
>>> * Enable user to disable loading default soundbank.
>>> Some users wanted to be able to open the synthesizer
>>> without the synthesizer loading the default soundbank.
>>>
>>> And then there was several other bug fixes:
>>> * Fix: Synchronization bug in n SoftSynthesizer.getDefaultSoundbank
>>> * Fix: AudioFloatInputStreamResampler.skip broken
>>> * Fix: RealTime scale/octave tuning doesn't affect sounding notes.
>>> * Fix: ModelByteBufferWaveTable.openStream().getFrameLength()
>>> returns incorrect values in some cases.
>>>
>>> Heres complete list of changes made:
>>> - Fix: Use AccessController.doPrivileged to access user settings
>>> and cached emergency soundbank.
>>> Affected File/Method: SoftSynthesizer.getDefaultSoundbank
>>> - Add: Support for user config file.
>>> Affected File/Method: SoftSynthesizer.getPropertyInfo
>>> - Add: Let SoftSynthesizer.getPropertyInfo use type conversion when needed.
>>> JTreg test added:
>>> /test/SoftSynthesizer/GetPropertyInfo
>>> - Add: Enable user to disable loading default soundbank.
>>> Affected File/Method: SoftSynthesizer.getPropertyInfo
>>> SoftSynthesizer.processPropertyInfo
>>> SoftSynthesizer.openStream
>>> JTreg test added:
>>> /test/SoftSynthesizer/TestDisableLoadDefaultSoundbank
>>> - Fix: SoftSynthesizer.getAvailableInstruments should
>>> always return instruments from default soundbank.
>>> Affected File/Method: SoftSynthesizer.getAvailableInstruments
>>> JTreg test added:
>>> /test/SoftSynthesizer/GetAvailableInstruments2
>>> - Fix: SoftSynthesizer.getLoadedInstruments should always
>>> return loaded instrument.
>>> Affected File/Method: SoftSynthesizer.getLoadedInstruments
>>> JTreg test added:
>>> /test/SoftSynthesizer/GetLoadedInstruments2
>>> JTreg tests fixed:
>>> /test/SoftSynthesizer/LoadAllInstruments
>>> /test/SoftSynthesizer/LoadInstrument
>>> /test/SoftSynthesizer/LoadInstruments
>>> /test/SoftSynthesizer/RemapInstrument
>>> * this test never worked correctly
>>> /test/SoftSynthesizer/UnloadAllInstruments
>>> /test/SoftSynthesizer/UnloadInstrument
>>> /test/SoftSynthesizer/UnloadInstruments
>>> - Fix: SoftSynthesizer.getDefaultSoundbank is not properly synchronized.
>>> Affected File/Method: SoftSynthesizer.getDefaultSoundbank
>>> - Optimization: Indexed version of ModelStandardDirector added.
>>> - Optimization: Fully unload instruments by setting null to unused values.
>>> Affected Files: SoftVoice, SoftChannel, SoftSynthesizer
>>> - Optimization: Prevent unnecessary lookup of instrument object
>>> on pseudo program change (when bank and program is unchanged)
>>> Affected File/Method: SoftChannel.programChange
>>> - Fix: AudioFloatInputStreamResampler.skip broken.
>>> (Gervill Bug Issues 5)
>>> see: https://gervill.dev.java.net/issues/show_bug.cgi?id=5
>>> Affected File/Method:
>>> AudioFloatFormatConverter.AudioFloatInputStreamResampler.skip
>>> JTreg test added:
>>> /test/AudioFloatFormatConverter/SkipTest
>>> - Fix: ModelByteBufferWaveTable.openStream().getFrameLength()
>>> returns incorrect values in some cases.
>>> (Gervill Bug Issues 4)
>>> see: https://gervill.dev.java.net/issues/show_bug.cgi?id=4
>>> Affected File/Method: ModelByteBufferWaveTable.openStream
>>> JTreg test added:
>>> /test/ModelByteBufferWavetable/OpenStream
>>> - Fix: RealTime scale/octave tuning doesn't affect sounding notes.
>>> Affected File/Method: SoftVoice.updateTuning
>>> JTreg test added:
>>> /test/ModelByteBufferWavetable/OpenStream
>>>
More information about the jdk6-dev
mailing list