Re: DTMF API completed

Sorry, I missed this DTMF thread (and must also take a look to the PR by
Robin). I won't be able to do it until next week.

BTW if you both have agreed on the perfect DTMF API (I don't know since I
missed the thread), could you please provide a Pull Request in Github?



2013/9/11 Martin Thomson <martin.thomson@skype.net>

>  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
>
>
>
>
> ****
>
>   ****
>
> *Iñaki Baz Castillo* <ibc@aliax.net>****
>
> 11 September, 2013 12:51 PM****
>
> 2013/9/11 Robin Raymond <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> ****
>
> ****
>
> *Robin Raymond* <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.
>
>
>
> ****
>
> ** **
>
> ****
>
> *Iñaki Baz Castillo* <ibc@aliax.net>****
>
> 11 September, 2013 10:35 AM****
>
> 2013/9/11 Robin Raymond <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> ****
>
> ****
>
> *Robin Raymond* <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.****
>
> ****
>
> *Iñaki Baz Castillo* <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
> ****
>
>
>
> ****
>
>


-- 
Iñaki Baz Castillo
<ibc@aliax.net>