Enable beamforming in Swanky

Enable beamforming in Swanky

BUG=405270
Committed: https://crrev.com/dd27f2fb61601a853487c6c90f800709a48d52bc
Cr-Commit-Position: refs/heads/master@{#313169}

[aluebs-chromium, 2015-01-16 02:21:16]

aluebs@chromium.org changed reviewers:
+ ajm@chromium.org, dalecurtis@chromium.org

[aluebs-chromium, 2015-01-16 02:21:17]

I am not sure if the bug I assigned is the correct one, since it says for Peach
Pi, but it still is the same approach.

[Pawel Osciak, 2015-01-16 02:26:20]

posciak@chromium.org changed reviewers:
+ posciak@chromium.org

[Pawel Osciak, 2015-01-16 02:26:21]

https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_audio_processor.cc:527: if (board ==
"peach_pi") {
Could we have a generic commandline flag for beam forming and set it  via board
use in chromium commandline builder instead of in Chrome please?

Please see examples for IsBoard() in
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/libchromeos/chr....

[aluebs-chromium, 2015-01-21 19:18:22]

https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_audio_processor.cc:527: if (board ==
"peach_pi") {
On 2015/01/16 02:26:21, Pawel Osciak wrote:
> Could we have a generic commandline flag for beam forming and set it  via
board
> use in chromium commandline builder instead of in Chrome please?
> 
> Please see examples for IsBoard() in
>
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/libchromeos/chr....

I would be happy to do it that way. My only concern is how to pass in the
geometry of the microphone array. Do you know how we could achieve that?

[Pawel Osciak, 2015-01-22 01:56:53]

https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_audio_processor.cc:527: if (board ==
"peach_pi") {
On 2015/01/21 19:18:21, aluebs wrote:
> On 2015/01/16 02:26:21, Pawel Osciak wrote:
> > Could we have a generic commandline flag for beam forming and set it  via
> board
> > use in chromium commandline builder instead of in Chrome please?
> > 
> > Please see examples for IsBoard() in
> >
>
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/libchromeos/chr....
> 
> I would be happy to do it that way. My only concern is how to pass in the
> geometry of the microphone array. Do you know how we could achieve that?

Is there an API you can query to get it?

[aluebs-chromium, 2015-01-22 01:59:08]

https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_audio_processor.cc:527: if (board ==
"peach_pi") {
On 2015/01/22 01:56:52, Pawel Osciak wrote:
> On 2015/01/21 19:18:21, aluebs wrote:
> > On 2015/01/16 02:26:21, Pawel Osciak wrote:
> > > Could we have a generic commandline flag for beam forming and set it  via
> > board
> > > use in chromium commandline builder instead of in Chrome please?
> > > 
> > > Please see examples for IsBoard() in
> > >
> >
>
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/libchromeos/chr....
> > 
> > I would be happy to do it that way. My only concern is how to pass in the
> > geometry of the microphone array. Do you know how we could achieve that?
> 
> Is there an API you can query to get it?

No, they depend on the device and they are hard-coded here. But I am happy to
have a better solution.

[Pawel Osciak, 2015-01-22 02:04:57]

On 2015/01/22 01:59:08, aluebs wrote:
>
https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
> File content/renderer/media/media_stream_audio_processor.cc (right):
> 
>
https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
> content/renderer/media/media_stream_audio_processor.cc:527: if (board ==
> "peach_pi") {
> On 2015/01/22 01:56:52, Pawel Osciak wrote:
> > On 2015/01/21 19:18:21, aluebs wrote:
> > > On 2015/01/16 02:26:21, Pawel Osciak wrote:
> > > > Could we have a generic commandline flag for beam forming and set it 
via
> > > board
> > > > use in chromium commandline builder instead of in Chrome please?
> > > > 
> > > > Please see examples for IsBoard() in
> > > >
> > >
> >
>
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/libchromeos/chr....
> > > 
> > > I would be happy to do it that way. My only concern is how to pass in the
> > > geometry of the microphone array. Do you know how we could achieve that?
> > 
> > Is there an API you can query to get it?
> 
> No, they depend on the device and they are hard-coded here. But I am happy to
> have a better solution.

Hardcoding things in Chrome per board like this is definitely not a good idea if
it can be avoided. In the file above there are some additional parameters
hardcoded as well in commandline parameters, you could do a similar thing, this
is probably the minimum that should be done instead of this. Ideally though you
should have an API to query the hardware, long term.

[aluebs-chromium, 2015-01-22 22:41:43]

On 2015/01/22 02:04:57, Pawel Osciak wrote:
> On 2015/01/22 01:59:08, aluebs wrote:
> >
>
https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
> > File content/renderer/media/media_stream_audio_processor.cc (right):
> > 
> >
>
https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
> > content/renderer/media/media_stream_audio_processor.cc:527: if (board ==
> > "peach_pi") {
> > On 2015/01/22 01:56:52, Pawel Osciak wrote:
> > > On 2015/01/21 19:18:21, aluebs wrote:
> > > > On 2015/01/16 02:26:21, Pawel Osciak wrote:
> > > > > Could we have a generic commandline flag for beam forming and set it 
> via
> > > > board
> > > > > use in chromium commandline builder instead of in Chrome please?
> > > > > 
> > > > > Please see examples for IsBoard() in
> > > > >
> > > >
> > >
> >
>
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/libchromeos/chr....
> > > > 
> > > > I would be happy to do it that way. My only concern is how to pass in
the
> > > > geometry of the microphone array. Do you know how we could achieve that?
> > > 
> > > Is there an API you can query to get it?
> > 
> > No, they depend on the device and they are hard-coded here. But I am happy
to
> > have a better solution.
> 
> Hardcoding things in Chrome per board like this is definitely not a good idea
if
> it can be avoided. In the file above there are some additional parameters
> hardcoded as well in commandline parameters, you could do a similar thing,
this
> is probably the minimum that should be done instead of this. Ideally though
you
> should have an API to query the hardware, long term.

I agree that this is not the place to hard-code the geometry, but also don't
like to pass it in as a command line parameter, since then I need to parse the
string to get a 2d float array. What do you think if I land this CL enabling
beamforming on  Swanky and then work adding the ChromiumOS API to get the
geometry? I created a bug to track this here:
https://code.google.com/p/chromium/issues/detail?id=451188 . Any suggestion is
appreciated, since I never worked on the ChromiumOS before.

[Pawel Osciak, 2015-01-23 00:08:33]

On 2015/01/22 22:41:43, aluebs wrote:
> On 2015/01/22 02:04:57, Pawel Osciak wrote:
> > On 2015/01/22 01:59:08, aluebs wrote:
> > >
> >
>
https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
> > > File content/renderer/media/media_stream_audio_processor.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
> > > content/renderer/media/media_stream_audio_processor.cc:527: if (board ==
> > > "peach_pi") {
> > > On 2015/01/22 01:56:52, Pawel Osciak wrote:
> > > > On 2015/01/21 19:18:21, aluebs wrote:
> > > > > On 2015/01/16 02:26:21, Pawel Osciak wrote:
> > > > > > Could we have a generic commandline flag for beam forming and set it

> > via
> > > > > board
> > > > > > use in chromium commandline builder instead of in Chrome please?
> > > > > > 
> > > > > > Please see examples for IsBoard() in
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/libchromeos/chr....
> > > > > 
> > > > > I would be happy to do it that way. My only concern is how to pass in
> the
> > > > > geometry of the microphone array. Do you know how we could achieve
that?
> > > > 
> > > > Is there an API you can query to get it?
> > > 
> > > No, they depend on the device and they are hard-coded here. But I am happy
> to
> > > have a better solution.
> > 
> > Hardcoding things in Chrome per board like this is definitely not a good
idea
> if
> > it can be avoided. In the file above there are some additional parameters
> > hardcoded as well in commandline parameters, you could do a similar thing,
> this
> > is probably the minimum that should be done instead of this. Ideally though
> you
> > should have an API to query the hardware, long term.
> 
> I agree that this is not the place to hard-code the geometry, but also don't
> like to pass it in as a command line parameter, since then I need to parse the
> string to get a 2d float array. What do you think if I land this CL enabling
> beamforming on  Swanky and then work adding the ChromiumOS API to get the
> geometry? I created a bug to track this here:
> https://code.google.com/p/chromium/issues/detail?id=451188 . Any suggestion is
> appreciated, since I never worked on the ChromiumOS before.

Since I was just doing a drive-by, I think it'd be best to ask one of the OWNERS
here...

[aluebs-chromium, 2015-01-26 18:30:19]

On 2015/01/23 00:08:33, Pawel Osciak wrote:
> On 2015/01/22 22:41:43, aluebs wrote:
> > On 2015/01/22 02:04:57, Pawel Osciak wrote:
> > > On 2015/01/22 01:59:08, aluebs wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
> > > > File content/renderer/media/media_stream_audio_processor.cc (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/816353011/diff/1/content/renderer/media/media...
> > > > content/renderer/media/media_stream_audio_processor.cc:527: if (board ==
> > > > "peach_pi") {
> > > > On 2015/01/22 01:56:52, Pawel Osciak wrote:
> > > > > On 2015/01/21 19:18:21, aluebs wrote:
> > > > > > On 2015/01/16 02:26:21, Pawel Osciak wrote:
> > > > > > > Could we have a generic commandline flag for beam forming and set
it
> 
> > > via
> > > > > > board
> > > > > > > use in chromium commandline builder instead of in Chrome please?
> > > > > > > 
> > > > > > > Please see examples for IsBoard() in
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/libchromeos/chr....
> > > > > > 
> > > > > > I would be happy to do it that way. My only concern is how to pass
in
> > the
> > > > > > geometry of the microphone array. Do you know how we could achieve
> that?
> > > > > 
> > > > > Is there an API you can query to get it?
> > > > 
> > > > No, they depend on the device and they are hard-coded here. But I am
happy
> > to
> > > > have a better solution.
> > > 
> > > Hardcoding things in Chrome per board like this is definitely not a good
> idea
> > if
> > > it can be avoided. In the file above there are some additional parameters
> > > hardcoded as well in commandline parameters, you could do a similar thing,
> > this
> > > is probably the minimum that should be done instead of this. Ideally
though
> > you
> > > should have an API to query the hardware, long term.
> > 
> > I agree that this is not the place to hard-code the geometry, but also don't
> > like to pass it in as a command line parameter, since then I need to parse
the
> > string to get a 2d float array. What do you think if I land this CL enabling
> > beamforming on  Swanky and then work adding the ChromiumOS API to get the
> > geometry? I created a bug to track this here:
> > https://code.google.com/p/chromium/issues/detail?id=451188 . Any suggestion
is
> > appreciated, since I never worked on the ChromiumOS before.
> 
> Since I was just doing a drive-by, I think it'd be best to ask one of the
OWNERS
> here...

DaleCurtis, WDYT?

[DaleCurtis, 2015-01-26 20:07:11]

Cleanup in a followup CL is fine with me.  lgtm

[ajm, 2015-01-26 20:59:06]

lgtm

[aluebs-chromium, 2015-01-26 22:49:11]

The CQ bit was checked by aluebs@chromium.org

[commit-bot: I haz the power, 2015-01-26 22:50:17]

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

[commit-bot: I haz the power, 2015-01-26 23:39:52]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-01-26 23:41:12]

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