Closed Bug 1126224 Opened 10 years ago Closed 7 years ago

[B2G] New audio channel type for VOIP call

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: alwu, Unassigned)

References

Details

Attachments

(2 files, 8 obsolete files)

Now Voip call use the "telephony" type for its media element, but it is different with the GSM call, so we want to distinguish them.

In new audio channel architecture design, the voip could be managed directly by the system app, but telephony couldn't. The state of telephony is accessed by TelephonyService.
Assignee: nobody → alwu
Depends on: 1113086
Attached patch Add new audio channel type - "voip" (obsolete) (deleted) — Splinter Review
Hi, Baku,
Could you give me some suggestions :)?

Due to different telephony sources use the same audio channel type would result some misunderstanding, so we decide to distinguish them.

The Voip call is using the MediaElement to implement, so we can access it by the BrowserElementAudioChannel. But the GSM call is coming from RIL, it doesn't have MediaElement. We access it by the TelephonySerive. Now we use the |SetPhoneState()| to create a audio agent, but this agent is not related to any window. Therefore, we can't control it by BrowserElementAudioChannel.

If we want to let this agent have a window, we must send a window reference to tell TelephonyService who is using the telephony channel, by some ways. (Maybe export a function from TelephonyService or AudioManager) 

I discuss this issue with Dominic and other telephony-team folks, they think that the telephony service shouldn't belong to any specific window, it's weird for them. Every apps can use TelephonyService, so it's difficult and strange to define the actually telephony users. 

Therefore, we decide to keep the RIL architecture, and use different type to distinguish the Voip & GSM call.
(btw, they all still connect to the "telephony" stream type of AudioSystem)

---

I already add the "voip" type in AudioChannel.idl, but the auto-generated binding c++ header file doesn't have this type. Did I miss something?

---

Thanks a lots :)
Attachment #8555196 - Flags: feedback?(amarchesini)
The apps can control the telephony using the 'Telephony' object (Telephony.webidl).
Can we add a nsIAudioChannelAgent there?
Then this can send messages to the TelephonyService to mute/unmute/volume when needed.

> I already add the "voip" type in AudioChannel.idl, but the auto-generated
> binding c++ header file doesn't have this type. Did I miss something?

You should edit dom/audiochannel/AudioChannelService.cpp too.
Hi, Baku,
If we add an AudioChannelAgent in the telephony object, that means we need to pass a window reference when we use it. 

There are some issues we need to discuss, 
(1) Is appropriate to add a agent in the telephony object?
(1) Is appropriate to use this way to control the telephony? 

That we start from the first point.
If we create the audio agent in the telephony object, that means there must have "someone" who is the owner of this agent. I think that is the same as giving the telephony ownership to some specific window. As I mentioned in comment1, the telephony guys don't want to see that.

The second point is, even we create the correct agent for the telephony, this agent doesn't have a callback object. So there wouldn't have any media elements to change their muted/volume when we change the state of the BrowserElementAudioChannel. 

I think that using this way to control the RIL-based telephony is little strange, isn't it?
Attached patch Add new audio channel type - "voip" (obsolete) (deleted) — Splinter Review
Add voip permission in the permissions table.
Attachment #8555196 - Attachment is obsolete: true
Attachment #8555196 - Flags: feedback?(amarchesini)
Attached patch WIP : change voip audio routing path (obsolete) (deleted) — Splinter Review
Hi, Wanye,
I need to change the default routing path of the voip from receiver to speaker, because I map it to the stream type "telephony".
But it doesn't work now.
Could you give me some advices? Do I miss something?
Thanks a lots :)
Flags: needinfo?(waychen)
Sorry for the wrong flag.
Attachment #8557000 - Attachment is obsolete: true
Flags: needinfo?(waychen)
Attachment #8557027 - Flags: feedback?(waychen)
> I think that using this way to control the RIL-based telephony is little
> strange, isn't it?

2 questions:

1. Where do we use the 'telephony' audiochannel?
If we don't use it to control the telephony object, we should remove it.

2. If the telephony should be window-less, why voip should have a window?
For the user point of view, a telephony call and a voip call should not be treated differently.
If BrowserElemenetAudioChannel can control the voip, but not the telephony, it seems wrong to me.
Flags: needinfo?(alwu)
As you says, it seems that I don't use the "telephony" audio channel at well. I only use it to notify the system app that telephony is starting or ending.

The second question, I don't know the answer yet. I treat telephony audio channel as differently just because it's belong to the RIL-based system. It seems not the same as the mediaElement-based system. I don't know whether we should take these system the same...

If we add a specific agent for the telephony, the gaia developer need to define the telephony start/end timing. Is it a well design for them? Now the apps don't need to handle the telephony state, the telephony service would be in charge all of it. 

But what you mentioned is also important. I need to discuss with telephony guys again.

So, the ideal thing to you is that we create the VOIP state, and the telephony state can also work?

Thanks a lots!!!!
Flags: needinfo?(alwu) → needinfo?(amarchesini)
Attached patch Change voip audio routing (obsolete) (deleted) — Splinter Review
Let the voip sound come out from the speaker.
Attachment #8557027 - Attachment is obsolete: true
Attachment #8557027 - Flags: feedback?(waychen)
Attached patch Change voip audio routing (obsolete) (deleted) — Splinter Review
Remove some debug logic.
Attachment #8558430 - Attachment is obsolete: true
Attached patch Change voip audio routing (obsolete) (deleted) — Splinter Review
The "voip" and GSM call are based on the different systems (RIL vs MediaElement). Therefore, I create a voip type to distinguish them.

There are two things in these patches.
First, mapping the "voip" type to the android stream type "voice call".
Second, changing its default audio routing path from the receiver to the speaker.

Hi, Baku,
Could you help me review this patch?
Very appreciate :)

ps. the telephony would still can be controlled by BrowserElementAudioChannel, I would field another bug to handle that.
Attachment #8558433 - Attachment is obsolete: true
Attachment #8558875 - Flags: review?(amarchesini)
Attached patch WIP : Change voip audio routing (obsolete) (deleted) — Splinter Review
Sorry for the mistake,
I still need to modify some parts, because I have not to follow the latest patch of BUG 1113086 yet.
Change it to WIP.
Attachment #8558875 - Attachment is obsolete: true
Attachment #8558875 - Flags: review?(amarchesini)
Attachment #8556949 - Attachment is obsolete: true
Attachment #8558903 - Flags: review?(amarchesini)
Attached patch Change voip audio routing (deleted) — Splinter Review
Hi, Baku,
Could you help me review these patches?
The introduction of these patches in on the comment 11.
Very appreciate!!!
Attachment #8558889 - Attachment is obsolete: true
Attachment #8558904 - Flags: review?(amarchesini)
Flags: needinfo?(amarchesini)
If we implement Telephony AudiochannelAgent, do we still need these patches?
Flags: needinfo?(alwu)
Ask Dominic for more details,
He is going to discuss this with UX.
Flags: needinfo?(alwu) → needinfo?(dkuo)
cc the owner of loop app. loop needs to change the type from telephony to voip after we have new AudioChannel.
Steven, thanks for letting us know about this new audio channel. This change is quite big and risky, not for the FxOS Hello app itself but also for some certificate Gaia apps such call screen app. Let me let you guys know what's the usage of the telephony audio channel in the FxOS Hello app. We implemented a policy for the usage of the telephony audio channel that allowed both the Hello and the call screen app  to handle interruptions between GSM/CDMA and Hello calls (WebRTC calls). Moreover we hacked a bit as well around the MozSpeakerManager API for the usage of the telephony audio channel because the Hello app must be able to switch audio output from built-in earpiece to speakers and vice versa. And of course, the last change I did around the telephony audio channel was to allow privileged app to use it so the same must happen with the voip audio channel. Those are the thing I envision that must behave the same with this new channel. I would recommend you guys to create a meta bug for tracking all the work that needs to be done and work together to avoid regressions in the functionality already implemented.
Hi, José,
We are implementing the new audio channel design now (Bug1113086 & Bug 1100822). The new design would change the control ability of audio channel from the gecko to the gaia. We need to distinguish the differences between voip and telephony in order to make system app can do the correct audio policy decision. 

And you mentioned some points, let me explain these under the new design.
(1) handle interruptions between GSM/CDMA and Hello calls (WebRTC calls)
The system app can control all audio channels, so it could know the new incoming WebRTC during the GSM/CDMA call. 

(2) hello app must be able to switch audio output from built-in earpiece to speakers and vice versa.
In this changeset, the default audio output of voip is out from the speaker, is that you want?
If you want to change the audio output during the app, you can use the telephony object to achieve that, right?

(3) allow privileged app to use it so the same must happen with the voip audio channel.
We can do that.

If I misunderstand something or you still have some questions,
Welcome to discuss it together!

Hope my response is helpful to you :)!
(In reply to Alastor Wu [:alwu] from comment #19)

Thanks for briefing how the new design is supposed to work.

I forgot to ask you something in my last comment; what's the priority for the new audio channel? After having a look at the patches it seems the priority of the voip audio channel is lower that the telephony one. That would mean that a GSM/CDMA call always will pause a WebRTC one. In current implementation both types of call compete for the usage of the telephony audio channel so they have the same priority. With what you guys are proposing GSM/CDMA call will pause WebRTC calls and that's wrong.
 
> And you mentioned some points, let me explain these under the new design.
> (1) handle interruptions between GSM/CDMA and Hello calls (WebRTC calls)
> The system app can control all audio channels, so it could know the new
> incoming WebRTC during the GSM/CDMA call. 

Well the system app controls everything, that's fine but how is the system app going to notify the Hello app about a new GSM/CDMA call interrupting a WebRTC one? (current implementation uses mozinterrupt{begin, end} events.
  
> (2) hello app must be able to switch audio output from built-in earpiece to
> speakers and vice versa.
> In this changeset, the default audio output of voip is out from the speaker,
> is that you want?
> If you want to change the audio output during the app, you can use the
> telephony object to achieve that, right?

Yes, through the MozSpeakerManager API the Hello app is able to switch between built-in earpiece and speaker while using the telephony audio channel. Are we going to be able to to that when using the new audio channel?
I'm still not 100% sure we need this new audioChannel. I agree with what you said about the new audiochannel policy...

in the future we can have maybe multiple voip apps running at the same way and we will have similar problems as we have with gsm-voip.
What about if all these apps use 'telephony' audiochannel, and the system app does the magic to mute, change the volume, etc etc?

What are the issues in this scenario? Thanks!
Flags: needinfo?(alwu)
(1) Priority & some usages explanations

We could discuss the priority in two places, Android and Gaia. The priority in Gecko is meaningless, because the gecko would not involve in the decision of the audio competing. 

The voip type is mapped to the stream type "telephony" in the AudioFlinger, so it could be regarded the same priority as the telephony in Android. The system could also treat the voip as same as the telephony, it depends on the gaia implementation.

You can still use the "mozinterrupt{begin, end}" to receive the notification. The speakerManager is also used as usual.

If these types are the same, why we need the new type?
Let me explain that on the following description.

---

(2) Telephony, but not be controlled by TelephonyAPI? 

The voip has its own media element, so it is non-related with the RIL-system. It could be controlled by the media element, but the telephony could not. It's a weird thing that the telephony channel does not related with the TelephonyAPI.

---

(3) Inconsistent usage

Everytime the audio is playback, there would be an audio agent to register this audio to the AudioChannelService. The agent would be created when the media element called the function, play().The telephony needs the special mechanism to achieve that, because it doesn't have the MediaElement/AudioContext/AudioNode ..e.t.c.

In the GSM usage, we need to call an extra function to ensure we create an audio agent for it. (See Bug 1129882). For example, this function is ownAudioChannel().

If both of voip and telephony use the same channel type, the situation would be like this..
 
In GSM call,
>callscreen app: (manifest: audio-channel-telephony)
>{
>  window.navigator.mozTelephony.ownAudioChannel();
>}

In Voip call,
>webrtc app: (manifest: audio-channel-telephony)
>{
>  var um = window.navigator.getUserMedia();
>  um.mozAudioChannelType = "telephony";
>}

Don't you think that is strange the same channel type has the different coding logic? 

If you force they to use the same logic, that means the voip audio channel would have two audioAgents which have totally different callback functions. One command would trigger two different set volume/muted behavior.

---

How do you think?
If there have any mistake, please correct me!
Wait for your reply :)
Flags: needinfo?(josea.olivera)
Flags: needinfo?(amarchesini)
Flags: needinfo?(alwu)
First of all, I think all of us agreed the GSM call and WebRTC call have the same priority, this is also confirmed in the ux spec in bug 961967(attachment 8541542 [details], page 11). The new audio channel api will let the system app to decide the channel priorities, which means they are all adjustable if ux wants to change any behaviours, so actually the priorities are not the topic we are focusing here.

The problem is the RIL api doesn't use media element to produce sounds, which means the app(should be Callscreen) uses the RIL api doesn't need to set mozAudioChannelType to some media element. This is confusing because |mozAudioChannelType = "telephony"| is not written in the gaia codebase. And if the voip app(like Loop) also wants the telephony channel, it should set its media element's mozAudioChannelType to "telephony", so they will become:

* Callscreen(GSM Call)
- Add |audio-channel-telephony| permission to manifest.webapp.
- No need to set mozAudioChannelType to "telephony".

* Loop(WebRTC call)
- Add |audio-channel-telephony| permission to manifest.webapp.
- Need to set mozAudioChannelType to "telephony".

Looks strange to developers, right?(Same as Alastor mentioned in comment 22)

So it's clear that the RIL audio and WebRTC audio are essentially different in gecko/gaia implementation, it should make sense to add the new |voip| channel type to distinguish from |telephony|, since telephony is probably only used in Callscreen and we don't even have a line for it. Does this make sense? thanks.
Flags: needinfo?(dkuo)
> So it's clear that the RIL audio and WebRTC audio are essentially different
> in gecko/gaia implementation, it should make sense to add the new |voip|
> channel type to distinguish from |telephony|, since telephony is probably
> only used in Callscreen and we don't even have a line for it. Does this make
> sense? thanks.

But it's also true that the we will not have any difference between telephony and voip from an AudioChannel policy point of view. So, if the main concern is the developer, we can improve our documentation, saying that, using a TelephonyCall object, you will implicitly using an 'telephony' audioChannel.

Plus, Telephony API is available for certified app only, so this telephony vs voip issue will happen only for advanced uses.

About the implicit audiochannel, we have a similar issue with camera. When the camera is used, it uses a 'publicnotification' audio channel by default and it cannot be changed.
Flags: needinfo?(amarchesini)
In the case of Radio FM is almost the same as Telephony, it's not clear for the developer which channel the API is using, and there is no way to 'play' with the Audio Stream (imagine that, as a developer, I want to do a Radio FM App with a cool frequency graph based on the music which is playing).
Would it be possible to move these APIs to a more 'webby' approach? So, for example, we could have something as:

navigator.mozFMRadio.enable(frequency).then(function(stream) {
// Connect to <audio> element with mozAudioChannelType='content' as Music App/Video App
});

Does it make sense?

On the other hand, after playing a lot with the conflicts between WebRTC and Telephony API when working in Firefox Hello, for me both channels are the same (both of them have the same rights, the same LIFO strategy...), so I can not see the point of having a different name for the same thing.
Well, as I can see in the previous comments there are two different options here: keeping things as they are right now or adding a new audio channel for the WebRTC calls. For the first option as it seems confusing for the user we should improve the docs and for the second one we need to develop it first and then to document it as well. I don't have an strong opinion on what option we should take. If by adding a new audio channel things get clearer I'm ok with that.
Flags: needinfo?(josea.olivera)
Attachment #8558903 - Flags: review?(amarchesini)
Comment on attachment 8558904 [details] [diff] [review]
Change voip audio routing

Because of without the consensus, I think we can put this issue on hold.
Even without the VOIP state, the gaia refactor can still be going on.
If we really need it in someday, we can re-discuss this issue at that time.
Attachment #8558904 - Flags: review?(amarchesini)
Assignee: alwu → nobody
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: