Details

Reviewers

cedric
vtorri
raster
an.kroitor
NikaWhite
FurryMyad
rimmed
t.naumenko
Jaehyun
bowonryu

Commits

rEFL7a98f617e94e: ecore_audio: a minimal template for playing sound on WINDOWS is added

Summary

Realization of audio data playback through WASAPI (Windows Audio Session API).(minimal)
WASAPI model:

Find a playback device (in the default system)
Register the client on the playback device.
Create a playback format for audio data.
Initialize the client with the created format, and access mode, ....
Take the object-rendering from the client for play the data stream.
Play data stream

ecore_audio model:

create a object for play the data stream(out object)
create a object to receive the data stream(in object)
register in the out-object the in-object
play data stream

Necessary:
Realize the ecore_audio object to play the data stream using the WASAPI model.

How implemented:

The object ecore_audio_out_wasapi is implemented
object ecore_audio_out_wasapi - the object constructor is find a playback device (in the default system)(WASAPI)
_ecore_audio_out_wasapi_ecore_audio_out_input_attach - register in the out-object the in-object

   3.1 Register the client on the playback device.(WASAPI)
   3.2 Create a playback format for audio data.(WASAPI)
   3.3 Initialize the client with the created format, and access mode, ....(WASAPI)

_write_cb - play data

4.1 Take the object-rendering from the client for play the data stream.(WASAPI)
4.2 Play data stream(WASAPI)

Diff Detail

Repository

rEFL core/efl

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

There are a very large number of changes, so older changes are hidden. Show Older Changes

Just a few remarks

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
72	Will be better if this variable will be initialized by some value.
81	If it is failed to receive service from client - no reason to continue. Just return here.

This revision now requires changes to proceed.Aug 2 2017, 4:27 AM

Please update commit message. In corresponding to the convention .

Sound good for me. This commit fix sound problem for Eflete on Windows

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
433	Cann't agree with you. https://phab.enlightenment.org/w/coding_convention/

spaces are deleted
is replaced by #ifdef on #if
added timeout interval to WaitForSingleObject

vtorri added inline comments.Aug 3 2017, 3:37 AM

src/lib/edje/edje_multisense.c
283 ↗	(On Diff #11884)	why having this (and also line 280) ? I would have said that ecore_audio would avoid such #ifdef this may be something outside the scope of that patch. In the case of the scope of that patch, maybe adding some #fidef at the beginning of the file and use a common code

i.furs edited the summary of this revision. (Show Details)Aug 3 2017, 7:40 AM

fix checking return code for WaitForSingleObject
added checking return code for GetChannelCount and SetAllVolumes
Moved Insert(#if HAVE_WASAPI .... #endif) to format-patch ->

0001-edje-on-play-sound-for-WINDOWS.patch1 KBDownload

. This will be another patch.

Please update code formatting for arithmetic operations for whole code.

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
185	Please replace with "switch" construction. This will make code more readable. Thanks.
205	Add parenthesis around multi var conditions: if ((a - b) < ((b / a) + c))...
208	Coding convention: ((bread / nBlockAlign))
209	Coding convention: ((bread % nBlockAlign) != 0)

This revision now requires changes to proceed.Aug 10 2017, 6:54 PM

changed EFL_ECORE_BUILD to EFL_ECORE_AUDIO_BUILD

structure 'if.. else ...' changed to 'switch .. case ..'
added parentheses and space in rule/formulas.
added check of values returning methods GetCurrentPadding, GetBuffer, ReleaseBuffer

Added Jaehyun Cho and Bowon Ryu

added define for moved from edje_multisense.c to Ecore_Audio.h for select the type of render. (D5096)

i.furs mentioned this in D5096: edje: moved from edje_multisense.c to Ecore_Audio.h for select the type of render..Aug 14 2017, 9:25 AM

Looks good with a few cosmetic change.

src/lib/ecore_audio/Ecore_Audio.h
56	Changing the order will break ABI here. Please put it just before LAST. Also state that it was @since 1.21.
src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
151	Would be nice for readability to have some spacing between declaration and code. This apply to every function.

This revision now requires changes to proceed.Aug 14 2017, 10:07 AM

vtorri added inline comments.Aug 14 2017, 11:29 PM

src/lib/ecore_audio/Ecore_Audio.h
33	still missing this one
src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
156	still wanting an answer to my question

NikaWhite added inline comments.Aug 15 2017, 1:33 AM

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
156	@i.furs already changed the infinity waiting time by the waiting with some timeout WaitForSingleObject(_pd->event, dwMilliseconds). It is not perfect, because there possible that whole EFL application still could be freezed for this timeout. But this kind of solution I like more. And I am curious about your suggestion to use ecore main loop here. Do you have any idea how to implement this? Should we are check status of event from object by every ecore main loop iteration? It is mean that event code should be managed somewhere in ecore (maybe inside ecore events or so). Could you please explain a bit detailed about you prorpose?

vtorri added inline comments.Aug 15 2017, 1:49 AM

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
156	use ecore_main_win32_handler_add() on _pd->event and the callback will run the code in case WAIT_OBJECT_0 so make the code async
225	is this comment meaning that you have to implement a del function ?

fix: EFL_ECORE_BUILD -> EFL_ECORE_AUDIO_BUILD
added spacing between declaration and code
moved position ECORE_AUDIO_TYPE_WASAPI in _Ecore_Audio_Type

added: ecore_main_win32_handler_add

NikaWhite requested changes to this revision.Aug 29 2017, 5:38 AM

NikaWhite added inline comments.

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
193	Looks like this comment not needed already.
197	Let discuss about this. The `_pd->event` created at time when client initialized. For that event added handler `ecore_main_win32_handler_add(_pd->event, _write_cb, _pd);` that call function _write_cb each time when event happens. So when line 196 reached, that means that event already happens and does not necessary "wait until the specified object is in the signaled state"(msdn). All this "waiting things" already done by _ecore_main_win32_objects_wait function. I suggest you to read source code of `ecore_main_win32_handler_add` and functions around that. The code placed inside EFL ecore_main module. In my opinion the WaitForSingleObject(_pd->event, dwMilliseconds) isn't need in your code at all.

This revision now requires changes to proceed.Aug 29 2017, 5:38 AM

WaitForSingleObject deleted

Harbormaster completed remote builds in B4404: Diff 12142.Aug 30 2017, 3:26 AM

This seems to have integrated all request made during the past review. Anything left to be done here ? I can't test, so who did test it ?

i have realized that there are some missing undef EAPI at the end of some headers

also i have remarked that static libs were added while they should not

otherwise, i will be able to test on 9 or 10 september

configure.ac
798	why is it needed ? it should be added only for ecore_audio
4041	why is it needed ? it should be added only for ecore_audio
src/lib/ecore_audio/ecore_audio_obj_in_sndfile.h
22	missing undef EAPI
src/lib/ecore_audio/ecore_audio_obj_in_tone.h
30–31	missing undef EAPI
src/lib/ecore_audio/ecore_audio_protected.h
29 ↗	(On Diff #12142)	missing undef EAPI

i.furs updated this revision to Diff 12157.Aug 31 2017, 9:57 AM

i.furs marked 6 inline comments as done.

This comment was removed by i.furs.

static libraries '-luuid -lwinmm -lksuser' moved in ECORE_AUDIO
in Ecore_Audio.h moved macro '#define EAPI(Ecore_Audio.h is entry point) with ecore_audio of files'

Harbormaster completed remote builds in B4417: Diff 12158.Aug 31 2017, 10:03 AM

Looks like all remarks fixed.
I checked how it works. Sounds in entry plays well. Playing sounds from edje require patch D5096.

you can try with etui (https://github.com/vtorri/etui) with a comic book, djvu, multipage tiff or pdf document. Just use left arrow to go before the first page, it should plays the sound that is played in terminology. This is what i will test when i go back from holydays

In D5029#87158, @vtorri wrote:

you can try with etui (https://github.com/vtorri/etui) with a comic book, djvu, multipage tiff or pdf document. Just use left arrow to go before the first page, it should plays the sound that is played in terminology. This is what i will test when i go back from holydays

I failed to cross-compile etui for windows. There a dependencies from pdf, dejavu etc libraries.
Anyway I think that there should not be a problems to play this bell sound.

i.furs marked 4 inline comments as done.Sep 13 2017, 10:19 AM

This comment was removed by i.furs.

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c
72	value count will be rewrite. _pd->volume->lpVtbl->GetChannelCount(_pd->volume, &count);
156	added ecore_main_win32_handler_add
174	added event_callback
src/lib/edje/edje_multisense.c
283 ↗	(On Diff #11884)	Moved Insert(#if HAVE_WASAPI .... #endif) to format-patch -> 0001-edje-on-play-sound-for-WINDOWS.patch1 KBDownload

ping

@vtorri Could you check this out ? I think we are near ready for integration.

I did:

git pull
arc patch D5029 --nobranch
arc patch D5096 --nobranch

make -j7 install and build fails :

lib/ecore_audio/ecore_audio_obj_out_wasapi.c: In function '_write_cb':
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:199:29: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
                   if (bread < (nBlockAlign * numAvailableFrames))
                             ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:226:38: error: 'ECORE_AUDIO_OUT_WASAPI_EVENT_STOP' undeclared (first use in this function)
    efl_event_callback_call(_pd->out, ECORE_AUDIO_OUT_WASAPI_EVENT_STOP, NULL);
                                      ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:226:38: note: each undeclared identifier is reported only once for each function it appears in
lib/ecore_audio/ecore_audio_obj_out_wasapi.c: In function '_ecore_audio_out_wasapi_ecore_audio_out_input_attach':
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:25:18: error: 'ECORE_AUDIO_OUT_WASAPI_CLASS' undeclared (first use in this function)
 #define MY_CLASS ECORE_AUDIO_OUT_WASAPI_CLASS
                  ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:502:71: note: in expansion of macro 'MY_CLASS'
    Eina_Bool ret = ecore_audio_obj_out_input_attach(efl_super(eo_obj, MY_CLASS), in);
                                                                       ^
In file included from lib/ecore_audio/ecore_audio_obj_out_wasapi.c:9:0:
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:511:35: error: 'ECORE_AUDIO_OUT_WASAPI_EVENT_STOP' undeclared (first use in this function)
    efl_event_callback_add(eo_obj, ECORE_AUDIO_OUT_WASAPI_EVENT_STOP, _close_cb, _pd);
                                   ^
../src/lib/eo/Eo.h:2011:41: note: in definition of macro 'efl_event_callback_add'
    efl_event_callback_priority_add(obj, desc, \
                                         ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c: In function '_ecore_audio_out_wasapi_ecore_audio_out_input_detach':
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:25:18: error: 'ECORE_AUDIO_OUT_WASAPI_CLASS' undeclared (first use in this function)
 #define MY_CLASS ECORE_AUDIO_OUT_WASAPI_CLASS
                  ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:523:71: note: in expansion of macro 'MY_CLASS'
    Eina_Bool ret = ecore_audio_obj_out_input_detach(efl_super(eo_obj, MY_CLASS), in);
                                                                       ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:526:38: error: 'ECORE_AUDIO_OUT_WASAPI_EVENT_STOP' undeclared (first use in this function)
    efl_event_callback_call(_pd->out, ECORE_AUDIO_OUT_WASAPI_EVENT_STOP, NULL);
                                      ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c: In function '_ecore_audio_out_wasapi_efl_object_constructor':
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:25:18: error: 'ECORE_AUDIO_OUT_WASAPI_CLASS' undeclared (first use in this function)
 #define MY_CLASS ECORE_AUDIO_OUT_WASAPI_CLASS
                  ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:544:52: note: in expansion of macro 'MY_CLASS'
         eo_obj = efl_constructor(efl_super(eo_obj, MY_CLASS));
                                                    ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c: In function '_ecore_audio_out_wasapi_efl_object_destructor':
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:25:18: error: 'ECORE_AUDIO_OUT_WASAPI_CLASS' undeclared (first use in this function)
 #define MY_CLASS ECORE_AUDIO_OUT_WASAPI_CLASS
                  ^
lib/ecore_audio/ecore_audio_obj_out_wasapi.c:593:37: note: in expansion of macro 'MY_CLASS'
    efl_destructor(efl_super(eo_obj, MY_CLASS));
                                     ^
  CC       lib/ecore_audio/lib_ecore_audio_libecore_audio_la-ecore_audio_sndfile_vio.lo
make[3]: *** [Makefile:31469: lib/ecore_audio/lib_ecore_audio_libecore_audio_la-ecore_audio_obj_out_wasapi.lo] Error 1
make[3]: *** Attente des tâches non terminées....
  CC       lib/eio/lib_eio_libeio_la-eio_dir.lo
make[2]: *** [Makefile:51710: install-recursive] Error 1
make[1]: *** [Makefile:52396: install] Error 2
make: *** [Makefile:2945: install-recursive] Error 1

checking efl-build:

-----------cross-build----------------
git pull #(commit - 56667a8)

git apply D5096.diff
git apply D5029.diff

./build.sh efl_upstream native efl
./build.sh efl_upstream x86_64 efl

build successful

-----------msys32----------------
git pull #(commit - 56667a8)

git apply D5096.diff
git apply D5029.diff

make -j7 && make -j7 install

build successful

@vtorri Could you try to check it again?

the entries in elm_test are playing sound. Not etui (there is certainly a problem with it)

So I would say it is good

Closed by commit rEFL7a98f617e94e: ecore_audio: a minimal template for playing sound on WINDOWS is added (authored by zvukzyshyny, committed by cedric). · Explain WhyOct 3 2017, 11:47 AM

This revision was automatically updated to reflect the committed changes.

i.furs mentioned this in T6154: Ecore_Audio.h is using undefined macros.Oct 11 2017, 10:50 AM

		Path
M		configure.ac (22 lines)
M		src/Makefile_Ecore_Audio.am (11 lines)
M		src/lib/ecore_audio/Ecore_Audio.h (50 lines)
M		src/lib/ecore_audio/ecore_audio_obj.h (22 lines)
M		src/lib/ecore_audio/ecore_audio_obj_in.h (23 lines)
M		src/lib/ecore_audio/ecore_audio_obj_in_sndfile.h (23 lines)
M		src/lib/ecore_audio/ecore_audio_obj_in_tone.h (23 lines)
M		src/lib/ecore_audio/ecore_audio_obj_out.h (23 lines)
M		src/lib/ecore_audio/ecore_audio_obj_out_pulse.h (23 lines)
M		src/lib/ecore_audio/ecore_audio_obj_out_sndfile.h (23 lines)
A	M	src/lib/ecore_audio/ecore_audio_obj_out_wasapi.h (22 lines)
A	M	src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c (609 lines)
A	M	src/lib/ecore_audio/ecore_audio_out_wasapi.eo (18 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	11810	4923945		Jul 18 2017, 3:24 AM	★	★
Diff 2	11884	7bb1472	spaces was delete	Aug 1 2017, 7:22 AM	★	★
Diff 3	11906	7bb1472	- ecore_audio: A minimal template for playing sound on WINDOWS is added	Aug 3 2017, 1:20 AM	★	★
Diff 4	11914	7bb1472	fix checking return code for WaitForSingleObject	Aug 3 2017, 11:07 AM	★	★
Diff 5	11980	7bb1472	changed EFL_ECORE_BUILD to EFL_ECORE_AUDIO_BUILD	Aug 11 2017, 1:05 AM	★	★
Diff 6	11981	7bb1472	structure 'if.. else ...' changed to 'switch .. case ..'	Aug 11 2017, 4:46 AM	★	★
Diff 7	11996	7bb1472	dded define for moved from edje_multisense.c to Ecore_Audio.h for select the…	Aug 14 2017, 9:17 AM	★	★
Diff 8	12006	7bb1472	fix: EFL_ECORE_BUILD -> EFL_ECORE_AUDIO_BUILD	Aug 15 2017, 4:05 AM	★	★
Diff 9	12045	7bb1472	added: ecore_main_win32_handler_add	Aug 21 2017, 5:54 AM	★	★
Diff 10	12142	2f53bdf	WaitForSingleObject deleted	Aug 30 2017, 3:26 AM	★	★
Diff 11	12157	2f53bdf	static libraries '-luuid -lwinmm -lksuser' moved in ECORE_AUDIO	Aug 31 2017, 9:57 AM	★	★
Diff 12	12158	2f53bdf	static libraries '-luuid -lwinmm -lksuser' moved in ECORE_AUDIO	Aug 31 2017, 10:02 AM	★	★
Diff 13	12428	fa2faff	rEFL7a98f617e94ef42787e06e685a8d5e7be34fff21	Oct 3 2017, 11:36 AM	★	★

ecore_audio: A minimal template for playing sound on WINDOWS is added
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 12428

configure.ac

src/Makefile_Ecore_Audio.am

src/lib/ecore_audio/Ecore_Audio.h

src/lib/ecore_audio/ecore_audio_obj.h

src/lib/ecore_audio/ecore_audio_obj_in.h

src/lib/ecore_audio/ecore_audio_obj_in_sndfile.h

src/lib/ecore_audio/ecore_audio_obj_in_tone.h

src/lib/ecore_audio/ecore_audio_obj_out.h

src/lib/ecore_audio/ecore_audio_obj_out_pulse.h

src/lib/ecore_audio/ecore_audio_obj_out_sndfile.h

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.h

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c

src/lib/ecore_audio/ecore_audio_out_wasapi.eo

ecore_audio: A minimal template for playing sound on WINDOWS is addedClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 12428

configure.ac

src/Makefile_Ecore_Audio.am

src/lib/ecore_audio/Ecore_Audio.h

src/lib/ecore_audio/ecore_audio_obj.h

src/lib/ecore_audio/ecore_audio_obj_in.h

src/lib/ecore_audio/ecore_audio_obj_in_sndfile.h

src/lib/ecore_audio/ecore_audio_obj_in_tone.h

src/lib/ecore_audio/ecore_audio_obj_out.h

src/lib/ecore_audio/ecore_audio_obj_out_pulse.h

src/lib/ecore_audio/ecore_audio_obj_out_sndfile.h

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.h

src/lib/ecore_audio/ecore_audio_obj_out_wasapi.c

src/lib/ecore_audio/ecore_audio_out_wasapi.eo

ecore_audio: A minimal template for playing sound on WINDOWS is added
ClosedPublic
Actions

Revision Contents
Changeset List