Page MenuHomePhabricator

Ecore_Audio: fix using undefined macros
ClosedPublic

Authored by i.furs on Oct 11 2017, 8:02 AM.

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.
i.furs created this revision.Oct 11 2017, 8:02 AM
vtorri added a subscriber: vtorri.Oct 11 2017, 12:41 PM

no, you are using macro that are defined by the configure of the EFL. So if Ecore_Audio.h is used in ANOTHER application, HAVE_WASAPI and HAVE_PULSE are not defined and

#if HAVE_WASAPI

is actually

#if

that is with nothing. In public headers you must not used such macro but only system macro, like (on Windows) :

#ifdef _WIN32
# include <ecore_audio_obj_out_wasapi.h>
# define ECORE_AUDIO_OUT_RENDER_ACTIVATED EINA_TRUE
# define ECORE_AUDIO_OUT_RENDER_CLASS ECORE_AUDIO_OUT_WASAPI_CLASS
# define ECORE_AUDIO_OUT_RENDER_EVENT_CONTEXT_FAIL ECORE_AUDIO_OUT_WASAPI_EVENT_CONTEXT_FAIL
#endif
vtorri requested changes to this revision.Oct 11 2017, 12:41 PM
This revision now requires changes to proceed.Oct 11 2017, 12:41 PM
i.furs updated this revision to Diff 12527.Oct 12 2017, 3:29 AM
This comment was removed by i.furs.
Harbormaster failed remote builds in B4707: Diff 12528!
i.furs added a subscriber: artem.popov.
i.furs edited the summary of this revision. (Show Details)

now the ifdef HAVE_PULSE problem remains. It is exactly the same problem

One question : is there only 2 possible backends for now (pulseaudio and Windows) ?

if yes, then # ifdef HAVE_PULSE is useless
if no, then follow what is done in Elementary.h.in

i.furs updated this revision to Diff 12532.Oct 12 2017, 8:56 AM
  • Ecore_Audio: fix using undefined macros
i.furs updated this revision to Diff 12533.EditedOct 12 2017, 8:56 AM

affects on D5311

cedric accepted this revision.Oct 13 2017, 12:17 PM
cedric requested changes to this revision.Oct 13 2017, 12:21 PM

Actually this doesn't apply.

This revision now requires changes to proceed.Oct 13 2017, 12:21 PM

How I understand is 2 backend 'audiopulse' and 'wasapi'. in object 'audiopulse' used define HAVE_PULSE. In 'wasapi' no.
When used Ecore_Audio need to understanding, what type of object need to create.
I thinks to need unification of metod create object.
Variant 1: level configuration file of project(efl,...)
Variant 2: level Ecore_Audio of module
Variant 3: level user(edje_multisense ...)

How need to use Ecore_Audio?

i don't understand the question.

i.furs updated this revision to Diff 12812.EditedOct 30 2017, 10:49 AM

If this update is incorrect, please expand the problem or show how this problem may be reproduced. So, it's not quite clear the concept of using public headers for audio.

That looks better to me (the public headers don't change based on internal/platform config), but as I mentioned in another patch... I don't think exposing WSAPI or even PULSE in the public headers is good - especially as separate classes and in our future eo based api's because we do try and be portable. there can be rare reasons to expose a platform specific api (because you may need to glue in platform specific data/objects/types) sometimes, but this here is not the case. there just should be a single portable class with portable methods (and maybe feature detection if needed to know if some feature in the api can work or not on the target system).

imagine i'm writing an application in JS and it's now portable. with zero changes the same app runs on windows, osx, linux, tizen and other places... i am not going to have #ifdef's in it as i'm not compiling it, and making me runtime check if a wsapi class works or a pulse one or something else... is pretty silly. i know this is used by edje for audio, but it's ALSO a public api for audio access too.

This revision was automatically updated to reflect the committed changes.