Creating the audio_modem component

Pulls the audio handling out of copresence and into its own component, with no functional changes. This is part of a larger plan to make audio token exchange available directly, via a private API.

rkc@ and xiyuan@ for copresence review.
caitkp@ for component review.
thestig@ for chrome/browser/DEPS review.
jam@, avi@ for content/ DEPS review.
dalecurtis@ for media/ DEPS review.
andrew@webrtc for WavWriter DEPS review.

BUG=455823
Committed: https://crrev.com/a9408e1449c5f950c1923ea90d75d7b00eccd753
Cr-Commit-Position: refs/heads/master@{#316639}

[Charlie, 2015-02-07 01:13:09]

ckehoe@chromium.org changed reviewers:
+ caitkp@chromium.org, rkc@chromium.org, thakis@chromium.org,
xiyuan@chromium.org

[Charlie, 2015-02-07 01:24:37]

ckehoe@chromium.org changed reviewers:
+ andrew@webrtc.org, dalecurtis@chromium.org, jam@chromium.org

[Charlie, 2015-02-07 01:25:48]

Happy Weekend!

[rkc, 2015-02-07 01:44:36]

Can we call the component AudioModem or something that indicates audio? This
component 'uses' whispernet to broadcast/receive tokens, but isn't really
whispernet itself.

[Nico, 2015-02-07 02:00:15]

why is the bug private? why will the api be private?

[chromium-reviews, 2015-02-07 02:00:27]

It depends on what you mean by "Whispernet". If whispernet is a net(work),
the audio codec doesn't really do anything without the actual transmission
and reception provided here. There are other parts of the code that use
"whispernet" to refer specifically to the codec, but I hope to gradually
subsume those into the whispernet component.

AudioModem *is* what it's called on Android, but this naming presents a few
problems. First, it's not a general audio modem: even if you were to plug
in a different encoding library, the rest of it is designed to transmit
tokens on these two specific audio bands that have specific parameters
baked into the code. It's not just an audio modem, it's our special
audible/ultrasound modem designed to exchange short tokens. (In other
words, it's a whispernet that actually transmits and receives.) Second, the
naming is weird and repetitive. We'd have audio_modem::Modem,
audio:modem::AudioType, audio_modem::AudioToken, etc. whispernet::Modem is
a better description of the interface it exposes.


On Fri Feb 06 2015 at 5:44:37 PM <rkc@chromium.org> wrote:

> Can we call the component AudioModem or something that indicates audio?
> This
> component 'uses' whispernet to broadcast/receive tokens, but isn't really
> whispernet itself.
>
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2015-02-07 02:10:30]

For the long answer, ask abunner. The short answer is that this token
exchange technology is not yet ready for use by third parties.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[rkc, 2015-02-07 02:10:55]

On 2015/02/07 02:00:27, chromium-reviews wrote:
> It depends on what you mean by "Whispernet". If whispernet is a net(work),
> the audio codec doesn't really do anything without the actual transmission
> and reception provided here. There are other parts of the code that use
> "whispernet" to refer specifically to the codec, but I hope to gradually
> subsume those into the whispernet component.
> 
> AudioModem *is* what it's called on Android, but this naming presents a few
> problems. First, it's not a general audio modem: even if you were to plug
> in a different encoding library, the rest of it is designed to transmit
> tokens on these two specific audio bands that have specific parameters
> baked into the code. It's not just an audio modem, it's our special
> audible/ultrasound modem designed to exchange short tokens. (In other
> words, it's a whispernet that actually transmits and receives.) Second, the
> naming is weird and repetitive. We'd have audio_modem::Modem,
> audio:modem::AudioType, audio_modem::AudioToken, etc. whispernet::Modem is
> a better description of the interface it exposes.
> 
> 
> On Fri Feb 06 2015 at 5:44:37 PM <mailto:rkc@chromium.org> wrote:
> 
> > Can we call the component AudioModem or something that indicates audio?
> > This
> > component 'uses' whispernet to broadcast/receive tokens, but isn't really
> > whispernet itself.
> >
> >
> > https://codereview.chromium.org/865483005/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

This does almost the same thing that AudioModem does on Android, I am not sure
why this can't be named that. All the arguments against the name also apply to
Android.

[chromium-reviews, 2015-02-07 02:12:49]

Ok, I'll ask the Android side what they think.

On Fri, Feb 6, 2015, 6:10 PM null <rkc@chromium.org> wrote:

> On 2015/02/07 02:00:27, chromium-reviews wrote:
> > It depends on what you mean by "Whispernet". If whispernet is a
> net(work),
> > the audio codec doesn't really do anything without the actual
> transmission
> > and reception provided here. There are other parts of the code that use
> > "whispernet" to refer specifically to the codec, but I hope to gradually
> > subsume those into the whispernet component.
>
> > AudioModem *is* what it's called on Android, but this naming presents a
> > few
> > problems. First, it's not a general audio modem: even if you were to plug
> > in a different encoding library, the rest of it is designed to transmit
> > tokens on these two specific audio bands that have specific parameters
> > baked into the code. It's not just an audio modem, it's our special
> > audible/ultrasound modem designed to exchange short tokens. (In other
> > words, it's a whispernet that actually transmits and receives.) Second,
> > the
> > naming is weird and repetitive. We'd have audio_modem::Modem,
> > audio:modem::AudioType, audio_modem::AudioToken, etc. whispernet::Modem
> is
> > a better description of the interface it exposes.
>
>
> > On Fri Feb 06 2015 at 5:44:37 PM <mailto:rkc@chromium.org> wrote:
>
> > > Can we call the component AudioModem or something that indicates audio?
> > > This
> > > component 'uses' whispernet to broadcast/receive tokens, but isn't
> > really
> > > whispernet itself.
> > >
> > >
> > > https://codereview.chromium.org/865483005/
> > >
>
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>
> This does almost the same thing that AudioModem does on Android, I am not
> sure
> why this can't be named that. All the arguments against the name also apply
> to
> Android.
>
>
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2015-02-07 02:47:32]

Another question: Are any clients of copresence left after this? Can we kill
that component?

[chromium-reviews, 2015-02-07 04:42:33]

There are still a few clients of the copresence API, which uses the
copresence component. However that code path is deprecated.

On Fri, Feb 6, 2015, 6:47 PM null <thakis@chromium.org> wrote:

> Another question: Are any clients of copresence left after this? Can we
> kill
> that component?
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2015-02-07 17:11:21]

@rkc: how about calling the component whisper_modem?

On Fri, Feb 6, 2015, 8:42 PM Charlie Kehoe <ckehoe@google.com> wrote:

> There are still a few clients of the copresence API, which uses the
> copresence component. However that code path is deprecated.
>
> On Fri, Feb 6, 2015, 6:47 PM null <thakis@chromium.org> wrote:
>
>> Another question: Are any clients of copresence left after this? Can we
>> kill
>> that component?
>>
>> https://codereview.chromium.org/865483005/
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[rkc, 2015-02-07 18:31:00]

I would rather that we be consistent with Android. Plus the fact that we have a
delegate into the encoder, theoretically, we could use another codec do to the
encoding/decoding than whisper :)

[Nico, 2015-02-07 19:09:42]

On 2015/02/07 04:42:33, chromium-reviews wrote:
> There are still a few clients of the copresence API, which uses the
> copresence component. However that code path is deprecated.

In tree? Out of tree?

What's the timeline for removing that code? From what I understand, it has never
been really used, so the old code got added, added a bunch of binary size and
build time, and then got deprecated again. Now we're adding another library for
the same use case, while the old code is still around.

How about removing the old code first before trying again?

[rkc, 2015-02-07 20:17:01]

On 2015/02/07 19:09:42, Nico wrote:
> On 2015/02/07 04:42:33, chromium-reviews wrote:
> > There are still a few clients of the copresence API, which uses the
> > copresence component. However that code path is deprecated.
> 
> In tree? Out of tree?
> 
> What's the timeline for removing that code? From what I understand, it has
never
> been really used, so the old code got added, added a bunch of binary size and
> build time, and then got deprecated again. Now we're adding another library
for
> the same use case, while the old code is still around.
> 
> How about removing the old code first before trying again?

I am not sure where you are getting this from. The copresense API is currently
being used by at least two Google apps, and has been for development/dogfooding
for some time. When Charlie says that the API is deprecated, that means we
aren't sure on what this API will look like for third parties yet. The code will
still be used, the interface may change.

[chromium-reviews, 2015-02-09 21:10:07]

@thakis: the copresence component builds to less than a MB of binary size.
Additionally, this CL doesn't add anything, but just pulls the audio
handling out of copresence. The API to be built on top will be just a thin
wrapper.

@rkc: I can rename the component to audio_modem if you want. Can you remind
me why this should be in its own component though? Alternatively, we could
just expose the modem from within copresence, and put all of this into a
copresence/audio folder (since other mediums aren't planned anytime soon).


On Sat Feb 07 2015 at 12:17:02 PM <rkc@chromium.org> wrote:

> On 2015/02/07 19:09:42, Nico wrote:
> > On 2015/02/07 04:42:33, chromium-reviews wrote:
> > > There are still a few clients of the copresence API, which uses the
> > > copresence component. However that code path is deprecated.
>
> > In tree? Out of tree?
>
> > What's the timeline for removing that code? From what I understand, it
> has
> never
> > been really used, so the old code got added, added a bunch of binary size
> > and
> > build time, and then got deprecated again. Now we're adding another
> > library
> for
> > the same use case, while the old code is still around.
>
> > How about removing the old code first before trying again?
>
> I am not sure where you are getting this from. The copresense API is
> currently
> being used by at least two Google apps, and has been for
> development/dogfooding
> for some time. When Charlie says that the API is deprecated, that means we
> aren't sure on what this API will look like for third parties yet. The code
> will
> still be used, the interface may change.
>
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[rkc, 2015-02-09 21:13:10]

Its cleaner design :)

There will be more use cases for the Audio Modem component from Brillo
(bypassing the Copresence code), in which case we don't really want to build any
of the Copresence code with it.

[Charlie, 2015-02-09 23:23:03]

On 2015/02/09 21:13:10, Rahul Chaturvedi wrote:
> Its cleaner design :)
> 
> There will be more use cases for the Audio Modem component from Brillo
> (bypassing the Copresence code), in which case we don't really want to build
any
> of the Copresence code with it.

Ok, renamed. PTAL.

[Andrew MacDonald, 2015-02-10 04:27:55]

https://codereview.chromium.org/865483005/diff/60001/components/audio_modem/m...
File components/audio_modem/modem_impl.cc (right):

https://codereview.chromium.org/865483005/diff/60001/components/audio_modem/m...
components/audio_modem/modem_impl.cc:305: void ModemImpl::DumpToken(AudioType
audio_type,
Did you just want me to review this? Or is there some new use of WavWriter
elsewhere?

[Charlie, 2015-02-10 05:14:19]

https://codereview.chromium.org/865483005/diff/60001/components/audio_modem/m...
File components/audio_modem/modem_impl.cc (right):

https://codereview.chromium.org/865483005/diff/60001/components/audio_modem/m...
components/audio_modem/modem_impl.cc:305: void ModemImpl::DumpToken(AudioType
audio_type,
On 2015/02/10 04:27:55, andrew wrote:
> Did you just want me to review this? Or is there some new use of WavWriter
> elsewhere?

Yes, just this. Your LG is required because it's being moved to a new component.

[Andrew MacDonald, 2015-02-10 05:31:34]

lgtm on WavWriter use.

[DaleCurtis, 2015-02-11 00:59:29]

Doesn't look like you've changed any audio code, so I don't think you need my
approval. Let me know if there's something specific you wanted me to review.

[chromium-reviews, 2015-02-11 01:07:49]

Hi Dale,

I believe your LG is required since this is getting moved into a different
component, with its own DEPS file. Andrew was in a similar situation.

- Charlie


On Tue Feb 10 2015 at 4:59:30 PM <dalecurtis@chromium.org> wrote:

> Doesn't look like you've changed any audio code, so I don't think you need
> my
> approval. Let me know if there's something specific you wanted me to
> review.
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[DaleCurtis, 2015-02-11 01:23:27]

I see, the DEPS lgtm then.

[Cait (Slow), 2015-02-11 22:35:57]

components/ changes LGTM.

[rkc, 2015-02-12 15:40:14]

Looks like there is no functional change, just mechanical changes required to
separate out the component? Those changes look fine.

lgtm

[chromium-reviews, 2015-02-12 17:23:17]

Yep, for simplicity, these are basically all renaming changes.

thakis, jam: your LGs are still required for the new component's DEPS file.
Let me know if someone else would be better to review.


On Thu Feb 12 2015 at 7:40:16 AM <rkc@chromium.org> wrote:

> Looks like there is no functional change, just mechanical changes required
> to
> separate out the component? Those changes look fine.
>
> lgtm
>
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Charlie, 2015-02-13 01:54:28]

ckehoe@chromium.org changed reviewers:
+ thestig@chromium.org

[Charlie, 2015-02-13 01:54:28]

+thestig for base/ and chrome/browser/DEPS review.

[Lei Zhang, 2015-02-13 02:03:34]

On 2015/02/13 01:54:28, Charlie wrote:
> +thestig for base/ and chrome/browser/DEPS review.

lgtm, but there's no base/ changes.

[chromium-reviews, 2015-02-13 05:39:50]

Thanks. I guess technically "changes" is a misnomer. Many base/ usages have
been moved to a new component with a new DEPS file, thus requiring a new LG.


On Thu Feb 12 2015 at 6:03:35 PM <thestig@chromium.org> wrote:

> On 2015/02/13 01:54:28, Charlie wrote:
> > +thestig for base/ and chrome/browser/DEPS review.
>
> lgtm, but there's no base/ changes.
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Lei Zhang, 2015-02-13 05:55:32]

On 2015/02/13 05:39:50, chromium-reviews wrote:
> Thanks. I guess technically "changes" is a misnomer. Many base/ usages have
> been moved to a new component with a new DEPS file, thus requiring a new LG.

You don't actually need to add base dependencies. It's whitelisted in src/DEPS
for everyone to use.

[chromium-reviews, 2015-02-13 06:17:44]

Ah, good to know, thanks.

On Thu, Feb 12, 2015, 9:55 PM null <thestig@chromium.org> wrote:

> On 2015/02/13 05:39:50, chromium-reviews wrote:
> > Thanks. I guess technically "changes" is a misnomer. Many base/ usages
> > have
> > been moved to a new component with a new DEPS file, thus requiring a new
> > LG.
>
> You don't actually need to add base dependencies. It's whitelisted in
> src/DEPS
> for everyone to use.
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jam, 2015-02-13 16:56:01]

jam@chromium.org changed reviewers:
- jam@chromium.org

[jam, 2015-02-13 16:56:16]

there are no files in content/ so removing myself

[rkc, 2015-02-13 17:01:05]

The CQ bit was checked by rkc@chromium.org

[commit-bot: I haz the power, 2015-02-13 17:02:20]

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

[commit-bot: I haz the power, 2015-02-13 17:07:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-13 17:07:07]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Charlie, 2015-02-15 17:41:10]

New patchsets have been uploaded after l-g-t-m from
andrew@webrtc.org,dalecurtis@chromium.org,caitkp@chromium.org,rkc@chromium.or...

[chromium-reviews, 2015-02-15 17:51:16]

@jam: I need your approval since files using content/ have been moved to a
new component, with a new DEPS file:

> git cl presubmit
Loaded authentication cookies from
/usr/local/google/home/ckehoe/.codereview_upload_cookies
Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from OWNERS of dependencies added to DEPS:
    '+content/public',


On Fri Feb 13 2015 at 9:07:10 AM <commit-bot@chromium.org> wrote:

> Try jobs failed on following builders:
>    mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
> http://build.chromium.org/p/tryserver.chromium.mac/builders/
> mac_chromium_compile_dbg_ng/builds/23767)
>
> https://codereview.chromium.org/865483005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Charlie, 2015-02-15 17:54:39]

ckehoe@chromium.org changed reviewers:
+ jam@chromium.org

[Charlie, 2015-02-15 17:54:40]

R += jam again. Please see the presubmit failure.

[Charlie, 2015-02-15 19:01:35]

Patchset #6 (id:100001) has been deleted

[Charlie, 2015-02-17 17:51:09]

ckehoe@chromium.org changed reviewers:
+ avi@chromium.org

[Charlie, 2015-02-17 17:51:11]

R += avi. DEPS approval needed for content/, since we're moving a bunch of files
to a new component.

[Avi (use Gerrit), 2015-02-17 18:17:46]

On 2015/02/17 17:51:11, Charlie wrote:
> R += avi. DEPS approval needed for content/, since we're moving a bunch of
files
> to a new component.

LGTM

[Charlie, 2015-02-17 18:27:56]

The CQ bit was checked by ckehoe@chromium.org

[commit-bot: I haz the power, 2015-02-17 18:28:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/865483005/120001

[commit-bot: I haz the power, 2015-02-17 18:49:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-17 18:49:59]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Charlie, 2015-02-17 18:55:35]

New patchsets have been uploaded after l-g-t-m from avi@chromium.org

[Charlie, 2015-02-17 18:56:34]

The CQ bit was checked by ckehoe@chromium.org

[commit-bot: I haz the power, 2015-02-17 18:58:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/865483005/140001

[commit-bot: I haz the power, 2015-02-17 20:05:32]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-02-17 20:06:17]

Patchset 7 (id:??) landed as
https://crrev.com/a9408e1449c5f950c1923ea90d75d7b00eccd753
Cr-Commit-Position: refs/heads/master@{#316639}