Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed audio and MIDI drivers; encode device names as CP_UTF8 #1325

Conversation

pedrolcl
Copy link
Contributor

@pedrolcl pedrolcl commented Apr 29, 2024

This also sets unicode (utf8) for CLI program's console output. It should be the proper fix for #983 wasapi problems.
cc: @tsingakbar

Solves #1322

@derselbst
Copy link
Member

Thanks Pedro! @tsingakbar If you have some time, could you pls. run fluidsynth.exe -Q and let us know if the reported WASAPI device names still look correct? For convenience, you may use the fluidsynth-x64 prebuilt binaries: https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=10122&view=artifacts&pathAsName=false&type=publishedArtifacts

@pedrolcl
Copy link
Contributor Author

CC @nanitaro , please test as well

@nanitaro
Copy link

nanitaro commented Apr 30, 2024

CC @nanitaro , please test as well

I ran fluidsynth.exe -Q on a test version.
Japanese information was output without problems as in 2.3.5 (official release version).
All variants (x64,mingw-x64,x86) have been checked.
I will upload the output log just to be sure.
fluidsynth.txt

@pedrolcl pedrolcl marked this pull request as draft May 1, 2024 14:06
Unicode support enabled in CMake script and CLI utility
@pedrolcl pedrolcl changed the title Fixed wasapi driver; encode device names as CP_UTF8 Fixed audio and MIDI drivers; encode device names as CP_UTF8 May 1, 2024
@pedrolcl pedrolcl marked this pull request as ready for review May 1, 2024 21:07
Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you! A few minor comments.

src/drivers/fluid_dsound.c Outdated Show resolved Hide resolved
include/fluidsynth/settings.h Show resolved Hide resolved
src/drivers/fluid_dsound.c Outdated Show resolved Hide resolved
@pedrolcl
Copy link
Contributor Author

pedrolcl commented May 5, 2024

With these changes, the users may enjoy utf8 output and input when using cmd.exe (or other shells) in Windows. The input is relevant for Fluidsynth's own shell. And this would be enough for libfluidsynth.dll clients, like Qsynth.

But there is still a loose thread: the CLI program's command line arguments are only processed/converted into utf8 for file names, but not other arguments. For instance, this would not work:

C:> fluidsynth.exe -o audio.wasapi.device=GroßeStraßenlautsprecher

@derselbst
Copy link
Member

Sorry for the late response, busy schedule.

It looks good to me now, thanks! If you want and have some time feel free to also UTF8 convert the commandline args.

@nanitaro Would you mind running fluidsynth.exe -Q again with these binaries? @tsingakbar CC
https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=10135&view=artifacts&pathAsName=false&type=publishedArtifacts

If everything goes well, I would apply the commits onto the release/2.3 and then merge it back to master. But probably not before next weekend, though.

@nanitaro
Copy link

nanitaro commented May 10, 2024

@derselbst
I ran the new test binary.
I think there is no problem about outputting Japanese information.
fluidsynth_05112024.txt

Compared to the previous binary, the output result of the default device in shared and exclusive mode seems to have changed.
This time the result seems to be more accurate.

For your information, the performance and settings of the sound devices in the test environment are as follows
Default device = SPDIF interface (FX-AUDIO-DAC-X6)
Windows sound properties. Default format=2ch,24bit,96000Hz

@pedrolcl
Copy link
Contributor Author

After the commits on May 1st, not only WASAPI but also the other Windows drivers are now updated to use UTF8. There is no equivalent of -Q for the other drivers, but it should be possible to use the shell, starting fluidsynth.exe without the argument and...

> settings
[...]
> info midi.driver
Options: winmidi
> info midi.winmidi.device
[...]
> info audio.driver
Options: dsound, waveout, wasapi
[...]
>  info audio.dsound.device
>  info audio.waveout.device
>  info audio.wasapi.device

The output of all these commands should be encoded now in UTF8.

Copy link

sonarcloud bot commented May 11, 2024

Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just applied the commit to the release/2.3 branch and merged it to master. Thanks @pedrolcl for the fix and @nanitaro for the tests!

@derselbst derselbst closed this May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants