Remove Channel::SetRingbackTone() and Channel::PlayRingbackTone(), and the code beneath it (within …

Remove Channel::SetRingbackTone() and Channel::PlayRingbackTone(), and the code beneath it (within libjingle).

BUG=webrtc:4690
TBR=juberti

Committed: https://crrev.com/22011c1b54021ec9a2b4885519e5ce995b1300a2
Cr-Commit-Position: refs/heads/master@{#10011}

[the sun, 2015-09-02 12:14:54]

solenberg@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[pthatcher1, 2015-09-02 21:03:48]

lgtm, with reservation

I had a similar CL a while back, but Justin thought we might want to use this at
some point in native mobile applications.

Can you check with Justin and Tina to make sure we'll never use this?

[the sun, 2015-09-03 07:56:54]

On 2015/09/02 21:03:48, pthatcher1 wrote:
> lgtm, with reservation
> 
> I had a similar CL a while back, but Justin thought we might want to use this
at
> some point in native mobile applications.
> 
> Can you check with Justin and Tina to make sure we'll never use this?

Certainly!

[the sun, 2015-09-03 07:57:22]

solenberg@webrtc.org changed reviewers:
+ juberti@webrtc.org, tina.legrand@webrtc.org

[the sun, 2015-09-03 08:24:45]

On 2015/09/03 07:56:54, the sun wrote:
> On 2015/09/02 21:03:48, pthatcher1 wrote:
> > lgtm, with reservation
> > 
> > I had a similar CL a while back, but Justin thought we might want to use
this
> at
> > some point in native mobile applications.
> > 
> > Can you check with Justin and Tina to make sure we'll never use this?
> 
> Certainly!

Justin, Tina: AFAICT this code is not used by any current libjingle clients.

For mobile, I would assume it is easy enough to use one of the native
play-single-shot-sound APIs for the ring tone, instead of hooking up jni thunks
and whatnot to be able to do it through libjingle.

[the sun, 2015-09-03 08:57:41]

On 2015/09/03 08:24:45, the sun wrote:
> On 2015/09/03 07:56:54, the sun wrote:
> > On 2015/09/02 21:03:48, pthatcher1 wrote:
> > > lgtm, with reservation
> > > 
> > > I had a similar CL a while back, but Justin thought we might want to use
> this
> > at
> > > some point in native mobile applications.
> > > 
> > > Can you check with Justin and Tina to make sure we'll never use this?
> > 
> > Certainly!
> 
> Justin, Tina: AFAICT this code is not used by any current libjingle clients.
> 
> For mobile, I would assume it is easy enough to use one of the native
> play-single-shot-sound APIs for the ring tone, instead of hooking up jni
thunks
> and whatnot to be able to do it through libjingle.

Ah, sorry, the reason is of course AEC. But then I would argue we'd not only
want ring tones added to the APM reverse path, but any other sound we can get
at, including other notifier sounds (and ideally system sounds too). So exposing
a PlayRingtone API is too specific, and nevertheless it has nothing to do with
particular VoE receive channels.

Instead, the ADM should offer a callback hook for obtaining the audio output
stream for use by the AEC. It may be possible on some platforms to get at the
actual ready-for-output stream; it can be fed back through this callback.
Realizing that is likely a bit of a moonshot, the next best thing would be to at
least make sure all app notifier sounds go to the AEC. For this it would be easy
to introduce a SimpleSoundPlayer class which can be plugged into the ADM as an
additional data source.

[the sun, 2015-09-10 12:23:42]

On 2015/09/03 08:57:41, the sun wrote:
> On 2015/09/03 08:24:45, the sun wrote:
> > On 2015/09/03 07:56:54, the sun wrote:
> > > On 2015/09/02 21:03:48, pthatcher1 wrote:
> > > > lgtm, with reservation
> > > > 
> > > > I had a similar CL a while back, but Justin thought we might want to use
> > this
> > > at
> > > > some point in native mobile applications.
> > > > 
> > > > Can you check with Justin and Tina to make sure we'll never use this?
> > > 
> > > Certainly!
> > 
> > Justin, Tina: AFAICT this code is not used by any current libjingle clients.
> > 
> > For mobile, I would assume it is easy enough to use one of the native
> > play-single-shot-sound APIs for the ring tone, instead of hooking up jni
> thunks
> > and whatnot to be able to do it through libjingle.
> 
> Ah, sorry, the reason is of course AEC. But then I would argue we'd not only
> want ring tones added to the APM reverse path, but any other sound we can get
> at, including other notifier sounds (and ideally system sounds too). So
exposing
> a PlayRingtone API is too specific, and nevertheless it has nothing to do with
> particular VoE receive channels.
> 
> Instead, the ADM should offer a callback hook for obtaining the audio output
> stream for use by the AEC. It may be possible on some platforms to get at the
> actual ready-for-output stream; it can be fed back through this callback.
> Realizing that is likely a bit of a moonshot, the next best thing would be to
at
> least make sure all app notifier sounds go to the AEC. For this it would be
easy
> to introduce a SimpleSoundPlayer class which can be plugged into the ADM as an
> additional data source.

Ping...

[the sun, 2015-09-10 12:24:22]

Justin, Tina, do you have any input?

[the sun, 2015-09-15 13:52:15]

On 2015/09/10 12:24:22, the sun wrote:
> Justin, Tina, do you have any input?

Ping...!

[the sun, 2015-09-17 13:39:41]

On 2015/09/15 13:52:15, the sun wrote:
> On 2015/09/10 12:24:22, the sun wrote:
> > Justin, Tina, do you have any input?
> 
> Ping...!

Ping......!

[pthatcher1, 2015-09-18 21:31:14]

On 2015/09/17 13:39:41, the sun wrote:
> On 2015/09/15 13:52:15, the sun wrote:
> > On 2015/09/10 12:24:22, the sun wrote:
> > > Justin, Tina, do you have any input?
> > 
> > Ping...!
> 
> Ping......!

Maybe we set a deadline for this, like "if no one objects, we'll submit it by
9/22, and if anyone objects after that we can revert if necessary".

[tlegrand-webrtc, 2015-09-20 06:49:40]

On 2015/09/18 21:31:14, pthatcher1 wrote:
> On 2015/09/17 13:39:41, the sun wrote:
> > On 2015/09/15 13:52:15, the sun wrote:
> > > On 2015/09/10 12:24:22, the sun wrote:
> > > > Justin, Tina, do you have any input?
> > > 
> > > Ping...!
> > 
> > Ping......!
> 
> Maybe we set a deadline for this, like "if no one objects, we'll submit it by
> 9/22, and if anyone objects after that we can revert if necessary".

LGTM, really nice clean-up!

Sorry for not being responsive on the "pings". If they don't make an actual
sound, they are not catching my attention, I'm afraid. Too many unread emails.
Next time (yes, there is probably going to be one), feel free to ping me using
some sound-making tool. :)

[the sun, 2015-09-21 11:40:31]

On 2015/09/18 21:31:14, pthatcher1 wrote:
> On 2015/09/17 13:39:41, the sun wrote:
> > On 2015/09/15 13:52:15, the sun wrote:
> > > On 2015/09/10 12:24:22, the sun wrote:
> > > > Justin, Tina, do you have any input?
> > > 
> > > Ping...!
> > 
> > Ping......!
> 
> Maybe we set a deadline for this, like "if no one objects, we'll submit it by
> 9/22, and if anyone objects after that we can revert if necessary".

Good idea, thanks Peter.

[the sun, 2015-09-22 07:50:20]

The CQ bit was checked by solenberg@webrtc.org

[the sun, 2015-09-22 07:50:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, tina.legrand@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1325023005/#ps60001 (title:
"rebase")

[commit-bot: I haz the power, 2015-09-22 07:50:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1325023005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1325023005/60001

[commit-bot: I haz the power, 2015-09-22 10:12:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-22 10:13:00]

Patchset 4 (id:??) landed as
https://crrev.com/22011c1b54021ec9a2b4885519e5ce995b1300a2
Cr-Commit-Position: refs/heads/master@{#10011}