RE: DTMF API completed

The key here is that the main API doesn’t get to worry about this nasty DTMF stuff.

Robin’s edit has a few minor issues, and one that I think bears further discussion.  I’ll take the minor ones to github as notes on the pull request, they shouldn’t affect the substance of the discussion.  But I’d like to talk about the other…

I have a few concerns with the approach that Robin has taken with respect to signaling tones.  I don’t know if this is intentional.  As proposed, the only type of DTMF tone supported is the indefinite length one.  The API has startTone() and stopTone() methods.

This was discussed at some length in the WebRTC group and the decision was to do away with indefinite length tones.  Definite length tones are the only form supported in the WebRTC APIs.  For one, that means that you never have the problem where tones are turned on, but never turned off.  More specifically, it’s a whole lot easier to do things like send sequences of tones.  It also means that receiving tones is easier (you don’t need an end tone event).

I’d like to see the API be as simple as possible:

dtmfTrack.playTones(‘555#’, toneLength = 70, toneGap = 50);
dtmfTrack.ontone = function(e) {
   assert(e.tone == ‘#’, e.duration >= 70);
};

There’s no need for anything more than that.

I haven’t shared this before, but I’ve attached the draft I made for DTMF.  It includes startTime and endTone methods, but I’ve since been convinced that these are unnecessary.  Other than that, I stand by the proposal.  I note that it’s also very close to what Robin has proposed.

From: Robin Raymond [mailto:robin@hookflash.com]
Sent: Wednesday, 11 September, 2013 11:29
To: Iñaki Baz Castillo
Cc: public-orca
Subject: Re: DTMF API completed


I'm not adding it to the base audio track. I'm adding it to a derived object extension. DTMFMediaStreamTrack "is a" MediaStreamTrack, but a specialized kind.

You don't need to have anything DTMF related in the core objects at all, including handlers. I compared your DTMF code before you made the change and that's not what I was meaning.

I have attached a patch propose exactly what I am suggesting:
https://github.com/openpeer/ortc/pull/13





[cid:image001.jpg@01CEAEFB.D5B7F430]
Iñaki Baz Castillo<mailto:ibc@aliax.net>
11 September, 2013 12:51 PM
2013/9/11 Robin Raymond <robin@hookflash.com<mailto:robin@hookflash.com>>

If you add "DTMF capabilities" (for both sending and receiving tones) to an audio track then you are "extending" /  "perverting" the audio MediaCapture MediaStreamTrack, am I wrong?

An anyhow, we need that the RTCTrackDescription for the audio+DTMF track shows the "dtmf" codec and thus, the RTCConnection needs to be told about this desire.  I would not like a new DTMF related method/event for MediaStreamTrack that is automatically detected by the WebRTC core stack so it produces a RTCTrackDescription which includes the DTMF stuff.

Well, we could get rid of the RTCConnection.addDtmfHandler() method and instead let the user creating an instance of RTCDTMFHandler class (by passing some audio track or media stream as constructor argument). And for the receiver side, the JS could create an instance of RTCDTMFHandler by passing as constructor argument the RTCTrackDescription which includes both audio and DTMF codecs. Then we would also get rid of the RTCConnection.onadddtmfhandler event.

And we could then remove RTCDTMFHandler and instead split it into RTCDTMFSender and RTCDTMFReceiver, which seems more appropriate with this approach.

Thoughts?


--
Iñaki Baz Castillo
<ibc@aliax.net<mailto:ibc@aliax.net>>
[cid:image002.jpg@01CEAEFB.D5B7F430]
Robin Raymond<mailto:robin@hookflash.com>
11 September, 2013 11:14 AM

I read Martin's comments and that's not what I get at all from his comments. Maybe I misunderstand what he's saying though.

As for Media Capture - I don't see how it perverts anything? Media Capture does not return that type of object and is unrelated.

I don't like dtmf objects being needlessly referenced into the main object as it therefor cannot be ignored. It should be "yet another extension". This is really a specialized type of audio track in many ways.

You can add a DTMF event handler even if it a separate object. You add the event handler to the dtmf media stream track object rather than the rtc connection. There's no issue or conflict.




[cid:image001.jpg@01CEAEFB.D5B7F430]
Iñaki Baz Castillo<mailto:ibc@aliax.net>
11 September, 2013 10:35 AM
2013/9/11 Robin Raymond <robin@hookflash.com<mailto:robin@hookflash.com>>

I like more the vision of Martin, in which DTMF is just an adicional codec over an existing audio track (see mails below). Having a specialized MediaStreamTrack was the previous attempt (before last commit) in which we had RTCDTMFTrack.

Now let's assume such an approach based on RTCDTMFTrack which is a kind of MediaStreamTrack. This is problematic as we do not want to pervert the MediaCapture MediaStreamTrack, do we? Do we want MediaStream.getDtmfTracks() method?

In our previous approach (before last commit) RTCDTMFTrack was not a kind of MediaStreamTrack at all, but a class totally separated from MediaCapture (this is, you cannot add a RTCDTMFTrack into a MediaStream and so on).

I strongly like the current approach in which DTMF is just a separate codec within an audio track ("dtmf" codec) which means that the RTCTrackDescription of an audio track includes an additional "dtmf" codec (along with a "opus" or "g711" codec).


> There's absolutely no reason IMHO to have the DTMF object have any methods in any of the core object.

We need a way to add a DTMF hander. We can add a method to RTCConnection (as we do now) or we can pervert the MediaCapture MediaStream / MediaStreamTrack with this legacy and artificial stuff.

We also need a way to receive DTMF. Yes, we could get rid of the RTCConnection.onadddtmfhandler event, and instead force the user to inspect the received RTCTrackDescription and, in case it is an audio track with "dtmf" codec, then create a RTCDTMFHandler instance by itself. I don't see too much benefit on that.


--
Iñaki Baz Castillo
<ibc@aliax.net<mailto:ibc@aliax.net>>
[cid:image002.jpg@01CEAEFB.D5B7F430]
Robin Raymond<mailto:robin@hookflash.com>
11 September, 2013 10:22 AM

DTMF should be a specialized MediaStreamTrack object, something like "DTMFMediaStreamTrack". There's absolutely no reason IMHO to have the DTMF object have any methods in any of the core object.
[cid:image001.jpg@01CEAEFB.D5B7F430]
Iñaki Baz Castillo<mailto:ibc@aliax.net>
10 September, 2013 11:33 AM
Hi Martin,

I've modified the API to include your proposal of DTMF as a separate
audio codec (rather than a separate track). Hope you like it:

https://github.com/openpeer/ortc/commit/fb467a7f6a54f7948f38c54e8e32f93a3439015a


Specially:

https://github.com/openpeer/ortc/commit/fb467a7f6a54f7948f38c54e8e32f93a3439015a#L0R707