Sound APIs on Windows by mattn · Pull Request #4522 · vim/vim

mattn · 2019-06-10T17:18:39Z

This is work in progress. Current status:

sound_playfile: done
sound_stop: done
sound_clear: done

src/sound.c

src/Make_cyg_ming.mak

src/sound.c

k-takata · 2019-06-11T03:06:55Z

@mattn I will create a patch for Make_mvc.mak later.

src/sound.c

k-takata · 2019-06-11T10:36:58Z

@mattn Could you import this patch?
https://gist.github.com/k-takata/4a8c42377c3256f1e29d26183b4b2a39

mattn · 2019-06-11T10:44:08Z

@k-takata Will do tonight. Thank you.

mattn · 2019-06-11T14:14:35Z

I applied review fixes. I noticed that implementing sound_playevent() is bits hard. The reasons are:

mciSendString does not allow alias name like "SystemAlert".
PlaySound() that can allow filename or "SystemAlert" does not work with callback.
PlaySound() can not stop sound indivisually.
"SystemAlert" is embeded in "mcires.dll".
Windows does not have XDG sound list.

brammool · 2019-06-11T14:36:21Z

I applied review fixes. I noticed that implementing sound_playevent() is bits hard. The reasons are: * mciSendString does not allow alias name like "SystemAlert". * PlaySound() that can allow filename or "SystemAlert" does not work with callback. * PlaySound() can not stop sound indivisually. * "SystemAlert" is embeded in "mcires.dll". * Windows does not have XDG sound list.

Thanks for working on this. Unfortunately, the list of sound events that is supported is unclear. There is a list from XDG, e.g. here: http://0pointer.de/public/sound-naming-spec.html But it contains many names that my Ubuntu system doesn't have, while the simple "bell" sound isn't listed. Does MS-Windows install some of these sounds, so that they can be played as sound files? Then we could map some event names to them. I rather not include sound files in the distribution, except some very basic ones like "bell". I assume plugins can include files in a way they play on any system with the +sound feature. .wav files should work, libcanberra also handles .ogg and other formats.

…

-- hundred-and-one symptoms of being an internet addict: 152. You find yourself falling for someone you've never seen or hardly know, but, boy can he/she TYPE!!!!!! /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

mattn · 2019-06-11T14:58:03Z

Does MS-Windows install some of these sounds, so that they can be played
as sound files? Then we could map some event names to them. I rather
not include sound files in the distribution, except some very basic ones
like "bell". I assume plugins can include files in a way they play on
any system with the +sound feature. .wav files should work, libcanberra
also handles .ogg and other formats.

Older version of Windows install them. But Windows 10 does not install them. All of sound resource are bundled in mcires.dll as resource file. PlaySound() API can play the resource sound. But PlaySound() is not used with callback as I mentioned in above.

brammool · 2019-06-11T16:47:34Z

>Does MS-Windows install some of these sounds, so that they can be >played as sound files? Then we could map some event names to them. >I rather not include sound files in the distribution, except some >very basic ones like "bell". I assume plugins can include files in a >way they play on any system with the +sound feature. .wav files >should work, libcanberra also handles .ogg and other formats. Older version of Windows install them. But Windows 10 does not install them. All of sound resource are bundled in mcires.dll as resource file. PlaySound() API can play the resource sound. But PlaySound() is not used with callback as I mentioned in above.

It looks like PlaySound() can play the sound synchronously with SND_SYNC, thus block until it's finished. If we can do this in a new thread, then it's possible to invoke the callback when finished. Is that doable?

…

-- Although the scythe isn't pre-eminent among the weapons of war, anyone who has been on the wrong end of, say, a peasants' revolt will know that in skilled hands it is fearsome. -- (Terry Pratchett, Mort) /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

k-takata · 2019-06-11T17:10:06Z

Is that doable?

I think it is possible, but the problem is that PlaySound() cannot stop the sounds individually.
It can stop all the sounds only. Another way is playing only one sound at once. If there is a new call of PlaySound(), stop the old sound.

src/sound.c

codecov-io · 2019-06-12T13:39:13Z

Codecov Report

Merging #4522 into master will increase coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4522      +/-   ##
==========================================
+ Coverage   80.65%   80.82%   +0.17%     
==========================================
  Files         111      111              
  Lines      143838   144338     +500     
==========================================
+ Hits       116011   116662     +651     
+ Misses      27827    27676     -151

Impacted Files	Coverage Δ
src/sound.c	`64.38% <ø> (ø)`	⬆️
src/if_xcmdsrv.c	`83.66% <0%> (-0.9%)`	⬇️
src/gui.c	`58.05% <0%> (-0.52%)`	⬇️
src/screen.c	`81.95% <0%> (-0.41%)`	⬇️
src/dict.c	`85.58% <0%> (-0.4%)`	⬇️
src/netbeans.c	`27.25% <0%> (-0.3%)`	⬇️
src/gui_gtk_x11.c	`49.16% <0%> (-0.25%)`	⬇️
src/ex_cmds.c	`82.18% <0%> (-0.25%)`	⬇️
src/os_unix.c	`60.63% <0%> (-0.05%)`	⬇️
src/evalfunc.c	`89.27% <0%> (-0.05%)`	⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12ee7ff...825abec. Read the comment docs.

mattn · 2019-06-12T14:28:41Z

Implemented sound_playevent() using PlaySound(). sound_playevent() return ID, but this can not be used for sound_stop().

src/sound.c

k-takata · 2019-06-13T14:19:16Z

@mattn Could you check this additional patch?
https://gist.github.com/k-takata/5fb38f305408a4bfa35221a5aec0f8ab

* Update document about sound_playevent() on Windows. * Some lines exceeded 80 columns. * Types of some variables were wrong.

k-takata · 2019-06-17T00:23:11Z

@mattn AppVeyor is failing. We need to fix the test.
https://ci.appveyor.com/project/chrisbra/vim/builds/25258938/job/2h4no54pbcv7fert#L3620

src/sound.c

k-takata · 2019-06-17T03:04:13Z

We've done. I think this is ready to be merged.

Problem: MS-Windows: no sound support. Solution: Add sound support for MS-Windows. (Yasuhiro Matsumoto, Ken Takata, closes vim#4522)

Problem: MS-Windows: no sound support. Solution: Add sound support for MS-Windows. (Yasuhiro Matsumoto, Ken Takata, closes vim/vim#4522) vim/vim@9b28352 ---- "sound" feature is N/A now but this updates "v:versionlong" docs. Co-authored-by: Bram Moolenaar <Bram@vim.org>

Problem: No numerical value for the patchlevel. Solution: Add v:versionlong. vim/vim@37df9a4 --- Restore "highest_patch()" solely for "v:versionlong". Copy/paste Test_vvar_scriptversion2() from patch 9.1.1540. It works without ":scriptversion 2". In general, if Vim's test works with ":scriptversion 1", just port it for additional coverage. Co-authored-by: Bram Moolenaar <Bram@vim.org> vim-patch:8.1.1565: MS-Windows: no sound support Problem: MS-Windows: no sound support. Solution: Add sound support for MS-Windows. (Yasuhiro Matsumoto, Ken Takata, closes vim/vim#4522) vim/vim@9b28352 ---- "sound" feature is N/A now but this updates "v:versionlong" docs. Co-authored-by: Bram Moolenaar <Bram@vim.org>

Problem: No numerical value for the patchlevel. Solution: Add v:versionlong. vim/vim@37df9a4 Restore "highest_patch()" solely for "v:versionlong". Copy/paste Test_vvar_scriptversion2() from patch 9.1.1540. It works without ":scriptversion 2". In general, if Vim's test works with ":scriptversion 1", just port it for additional coverage. --- vim-patch:8.1.1565: MS-Windows: no sound support Problem: MS-Windows: no sound support. Solution: Add sound support for MS-Windows. (Yasuhiro Matsumoto, Ken Takata, closes vim/vim#4522) vim/vim@9b28352 ---- "sound" feature is N/A now but this updates "v:versionlong" docs. Co-authored-by: Bram Moolenaar <Bram@vim.org>

mattn added 3 commits June 10, 2019 21:31

[WIP] Add sound on Windows

c9ca082

Merge branch 'master' into sound-windows

f7105e3

Implement sound_playfile, sound_stop, sound_clear

b6a6e9b

k-takata reviewed Jun 10, 2019

View reviewed changes

src/sound.c Outdated Show resolved Hide resolved

k-takata added the platform-windows label Jun 10, 2019

k-takata reviewed Jun 10, 2019

View reviewed changes

src/Make_cyg_ming.mak Outdated Show resolved Hide resolved

k-takata reviewed Jun 11, 2019

View reviewed changes

src/sound.c Outdated Show resolved Hide resolved

k-takata reviewed Jun 11, 2019

View reviewed changes

src/sound.c Outdated Show resolved Hide resolved

k-takata reviewed Jun 11, 2019

View reviewed changes

src/sound.c Show resolved Hide resolved

src/sound.c Outdated Show resolved Hide resolved

src/sound.c Outdated Show resolved Hide resolved

k-takata and others added 5 commits June 11, 2019 22:08

MSVC: Update makefile for sound

cf581b5

Use static

f46a726

HUGE or BIG

9830bd0

Use wide string

80f8f21

Add space

2ad1580

Merge branch 'master' into sound-windows

ab61278

k-takata reviewed Jun 11, 2019

View reviewed changes

src/sound.c Outdated Show resolved Hide resolved

src/sound.c Outdated Show resolved Hide resolved

src/sound.c Outdated Show resolved Hide resolved

src/sound.c Outdated Show resolved Hide resolved

src/sound.c Outdated Show resolved Hide resolved

src/sound.c Outdated Show resolved Hide resolved

Fix buffer size

fdc8ee8

mattn added 2 commits June 12, 2019 23:24

Implement sound_playevent()

17d3133

Update doc

e682b60

mattn changed the title ~~[WIP] Sound APIs on Windows~~ Sound APIs on Windows Jun 12, 2019

k-takata reviewed Jun 13, 2019

View reviewed changes

src/sound.c Show resolved Hide resolved

free(wp)

ac6e9b9

sound: Fix some issues

7c4d66d

* Update document about sound_playevent() on Windows. * Some lines exceeded 80 columns. * Types of some variables were wrong.

Skip test on Windows

32755ab

k-takata reviewed Jun 17, 2019

View reviewed changes

src/sound.c Outdated Show resolved Hide resolved

src/sound.c Outdated Show resolved Hide resolved

Fixes for PROTO

825abec

brammool closed this in 9b28352 Jun 17, 2019

manuelschiller pushed a commit to manuelschiller/vim that referenced this pull request Nov 10, 2019

patch 8.1.1565: MS-Windows: no sound support

fb325fa

Problem: MS-Windows: no sound support. Solution: Add sound support for MS-Windows. (Yasuhiro Matsumoto, Ken Takata, closes vim#4522)

This was referenced May 15, 2023

Windowsにて、sound_playfile() で再生した音声ファイルがvimを終了するまで開放されない vim-jp/issues#1413

Open

'shell' が pwsh.exe だと sound_playfile() で音が鳴らない vim-jp/issues#1412

Open

Uh oh!

Conversation

mattn commented Jun 10, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

k-takata commented Jun 11, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

k-takata commented Jun 11, 2019

Uh oh!

mattn commented Jun 11, 2019

Uh oh!

mattn commented Jun 11, 2019

Uh oh!

brammool commented Jun 11, 2019 via email

Uh oh!

mattn commented Jun 11, 2019

Uh oh!

brammool commented Jun 11, 2019 via email

Uh oh!

k-takata commented Jun 11, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-io commented Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mattn commented Jun 12, 2019

Uh oh!

Uh oh!

k-takata commented Jun 13, 2019

Uh oh!

k-takata commented Jun 17, 2019

Uh oh!

Uh oh!

Uh oh!

k-takata commented Jun 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Jun 12, 2019 •

edited

Loading