On Wed, 2 Aug 2023 03:23:33 GMT, Renjith Kannath Pariyangad <[email protected]>
wrote:
>> Hi Reviewers,
>>
>> Observations :
>> 1. Without com initialize if we access Mixer for recording, library loaded
>> invalid GUID and clipped description in windows(ID not found in registry).
>> With com initialization library load proper GUID (same as registry).
>> 2. For Play back device always loading proper device GUID irrespective of
>> com Initialization.
>>
>> Test:
>> Since screen lock and unlock workflow required for reproducing this issue,
>> did coupe of iteration of manual testing post fix and confirmed its
>> resolving the problem.
>> To reconfirm nothing is broken, executed all audio related test cases on
>> test bench post fix and all are green.
>>
>> Please review the changes and let me know your comments if any.
>>
>> Regards,
>> Renjith.
>
> Renjith Kannath Pariyangad has updated the pull request incrementally with
> one additional commit since the last revision:
>
> Added DS_unlockCache into failed Coinit case
Changes requested by aivanov (Reviewer).
src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_DirectSound.cpp
line 183:
> 181: INT32 cacheIndex;
> 182:
> 183: if (!DS_lockCache() || FAILED(::CoInitialize(NULL))) {
I believe this is incorrect: `::CoInitialize` may return `S_OK` or `S_FALSE`;
the value of `S_FALSE` indicates a success too but flags that COM is already
initialised. The only real error is `RPC_E_CHANGED_MODE` which also indicates
*COM is already initialised* but with a different threading model.
Thus, we can proceed in *all* the cases but we need to call `::CoUninitialize`
only if `::CoInitialize` returns `S_OK` or `S_FALSE`.
src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_DirectSound.cpp
line 183:
> 181: INT32 cacheIndex;
> 182:
> 183: if (!DS_lockCache() || FAILED(::CoInitialize(NULL))) {
As I [mentioned
before](https://github.com/openjdk/jdk/pull/14898#discussion_r1276539431), I
still think `::CoInitializeEx(NULL, COINIT_MULTITHREADED |
COINIT_DISABLE_OLE1DDE)` suits this case better than `::CoInitialize(NULL)`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/14898#pullrequestreview-1558847830
PR Review Comment: https://git.openjdk.org/jdk/pull/14898#discussion_r1281804647
PR Review Comment: https://git.openjdk.org/jdk/pull/14898#discussion_r1281807251
