[OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

Wed Mar 21 14:09:52 UTC 2018

On 2018-03-16 17:49, Alex Menkov wrote:
>
>
> On 03/15/2018 13:09, Magnus Ihse Bursie wrote:
>>
>>> 15 mars 2018 kl. 20:13 skrev Phil Race <philip.race at oracle.com>:
>>>
>>>
>>>> As far as I know the split was made to dynamically load 
>>>> ALSA/DirectSound stuff
>>>
>>> Yes, I think it is something like that for Linux
>>> ie if at runtime a dependent-but-not-essential .so was not
>>> installed it was not fatal. I don't know to what extent this is no 
>>> longer a
>>> possible issue, or one that matters.
>>
>> I have not heard of any mainstream Linux distro in years that was 
>> lacking ALSA.
>>
>> If ALSA was not present, will the libraries fall back to OSS, or will 
>> there be just no sound available?
>
> No sound.
> OSS support was dropped many years ago (IIRC in jdk7)
>
>> In any case, I think that whatever Linux distros we're targeting as 
>> supported, ALSA will be present.
>>
>> Alex, did I understand you correctly that in any case, a separate 
>> Windows library is always unnecessary, since we can rely on 
>> DirectAudio always being present in our supported versions of Windows?
>
> Yes, that's right.
> Windows always has DirectSound pre-installed and its version is 
> greater than required (IIRC javasoundds requires DirectX 5).
>
> For now failure of libjsound loading is fatal (see 
> com.sun.media.sound.Platform.loadLibraries()), loading of extra libs 
> is non-fatal.
> I believe libjsound loading failure should be made non-fatal, then all 
> the functionality will remain the same as we have now.

Ok.

Here is an updated webrev. I have made the following changes:
* libjavasoundalsa and libjavasoundds is now folded into the main 
libjavasound native library, so there's exactly one library built on all 
platforms.
* Loading of libjsound is made non-fatal.
* I have cleaned out all obvious parts of the code that handle multiple 
libraries. Since loading the native library is now a all-or-nothing 
situation, the checks for various subsystems have been turned into a 
generic check if the native library is loaded.

There is a lot of defines like USE_DSOUND which are always true. This 
could probably be cleaned up further, but it is not a build issue so I'm 
leaving that to the client team to handle.

I have tested the following:
  * COMPARE_BUILD shows me just the expected changes in the build.
  * On my linux machine, failure to load libjsound.so was not fatal.
  * I have looked for sound tests. I found the test/jdk/javax/sound 
suite, which was included in tier3. So I've run tier3 testing on all 
platforms using our internal test system, and all tests pass.

I don't know if there is any other tests I should run. If so, let me know.

Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8071469-cleanup-sound-libs/webrev.03

/Magnus

>
> --alex
>
>>
>> /Magnus
>>
>>>
>>> -phil.
>>>
>>>> On 03/15/2018 12:06 PM, Alex Menkov wrote:
>>>>
>>>>
>>>>> On 03/15/2018 11:44, Magnus Ihse Bursie wrote:
>>>>>> On 2018-03-15 18:23, Phil Race wrote:
>>>>>> I wondered if that might be the case since it was a "BSD" port .. 
>>>>>> using X11 ..
>>>>>>
>>>>>> Maybe we should be getting rid of them ?
>>>>> I agree, we should delete them. I just shuffled them around in the 
>>>>> hope that they would be useful for a potential future bsd port, 
>>>>> but if/when that happens, we can dig them out from mercurial.
>>>>>
>>>>> A short explanation of how the files moved. The sound library is 
>>>>> apparently composed of either a single library (solaris and 
>>>>> macosx) or two libraries (linux and windows). Two building blocks, 
>>>>> MIDI + ports and DirectAudio is used for all platforms, but they 
>>>>> go into either the main library (libjsound) or the helper library.
>>>>>
>>>>> For Windows, MIDI+Ports go into libjsound, and DirectAudio go into 
>>>>> libjsoundds. On Linux, MIDI+Ports and DirectAudio go into 
>>>>> libjsoundalsa. On Macosx and Solaris, MIDI+Ports and DirectAudio 
>>>>> go into the main libjsound.
>>>>>
>>>>> I have no idea why this split is necessary, but this is how the 
>>>>> libraries de facto is compiled, and the code needs to match that. 
>>>>> If it would be possible to move libjsoundds and libjsoundalsa into 
>>>>> libjsound directly, things would be greatly simplified.
>>>>
>>>> As far as I know the split was made to dynamically load 
>>>> ALSA/DirectSound stuff. If it's not available (or old unsupported 
>>>> version is installed), libjsound stuff continues to work (in 
>>>> pre-OpenJDK libjsound supported WaveIn/WaveOut on Windows and OSS 
>>>> on Linux).
>>>> For now Windows (DirectSound) libjsoundds stuff can be merged into 
>>>> libjsound, but I'm not sure we can rely on ALSA is always available 
>>>> on Linux (but most likely if ALSA is not available, libjsound does 
>>>> not provide any functionality, so I suppose libjsoundalsa stuff can 
>>>> be moved to libjsound as well)
>>>>
>>>> --alex
>>>>
>>>>>
>>>>> /Magnus
>>>>>
>>>>>>
>>>>>> -phil.
>>>>>>
>>>>>>> On 03/15/2018 10:21 AM, Erik Joelsson wrote:
>>>>>>> Digging a bit, those files came with the initial Macosx support. 
>>>>>>> It doesn't look like they were ever used.
>>>>>>>
>>>>>>> /Erik
>>>>>>>
>>>>>>>
>>>>>>>> On 2018-03-15 09:53, Phil Race wrote:
>>>>>>>> It is very hard to follow all the moved around files, but one 
>>>>>>>> thing
>>>>>>>> that sticks out is there is a "bsd" directory created and I can't
>>>>>>>> work out how the files in there are used.
>>>>>>>> If they are for a BSD port of OpenJDK where is rest of the 
>>>>>>>> support for that ?
>>>>>>>>
>>>>>>>>> On 03/15/2018 07:20 AM, Erik Joelsson wrote:
>>>>>>>>> Looks good to me. I tried cleaning this up before but failed 
>>>>>>>>> to find a reasonable split, but this seems like a good split 
>>>>>>>>> between common and library specific.
>>>>>>>>>
>>>>>>>>> /Erik
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On 2018-03-14 18:12, Magnus Ihse Bursie wrote:
>>>>>>>>>> I forgot to add the client mailing lists as recipients. 
>>>>>>>>>> Sorry. (Not sure if "sounds" belong to "awt" or "2d".)
>>>>>>>>
>>>>>>>> In fact, there is a sound-specific list, which I've added.
>>>>>>>>
>>>>>>>> -phil.
>>>>>>>>>>
>>>>>>>>>> /Magnus
>>>>>>>>>>
>>>>>>>>>>> On 2018-03-15 02:07, Magnus Ihse Bursie wrote:
>>>>>>>>>>>  From the bug description:
>>>>>>>>>>>
>>>>>>>>>>> Moving this to a separate bug from JDK-8055190. In 
>>>>>>>>>>> SoundLibraries.gmk, the source code splitting is not 
>>>>>>>>>>> complete. The directory libjsound is used to build not only 
>>>>>>>>>>> libjsound but libjsoundalsa and libjsoundds, and thus needs 
>>>>>>>>>>> a complex include/exclude system like before.
>>>>>>>>>>>
>>>>>>>>>>> I have tested this using COMPARE_BUILD. Windows and solaris 
>>>>>>>>>>> are completely clean. On macosx, there's a binary diff (but 
>>>>>>>>>>> nothing else) on libjsound.dylib. On linux, some offset 
>>>>>>>>>>> seems to have changed, which caused a slight change in 
>>>>>>>>>>> disass and fulldump for libjsound.so. I'm not quite sure 
>>>>>>>>>>> what's causing it, but I'm convinced it's harmless.
>>>>>>>>>>>
>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8071469
>>>>>>>>>>> WebRev: 
>>>>>>>>>>> http://cr.openjdk.java.net/~ihse/JDK-8071469-cleanup-sound-libs/webrev.01
>>>>>>>>>>>
>>>>>>>>>>> /Magnus
>>>
>>