New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 879967 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocked on:
issue 884893



Sign in to add a comment

CRAS: add echo reference iodev support

Project Member Reported by hychao@chromium.org, Sep 3

Issue description

Currently when we use WebRTC APM for echo cancellation in CRAS. The reverse stream being analyzed is:
(1) the mixed all output streams 
(2) post output DSP
passing to output iodev.

And this is not optimal for speakers with DSM. Because the final output audio will be changed in its DSP block.
CRAS should be able to open and read form the "echo reference" device, if machine driver supports it, and hook it for APM reverse analyze.

Implementation will be like:
(1) UCM can specify new SectionDevice. And the speaker can associate to it using "EchoReferenceDev" label
(2) Add support to a "server" stream, which is created without a client. The purpose of this stream is to open the echo reference iodev(input), and continuously read audio data from it.
(3) At speaker open event, possibly create a server stream and pins to the echo reference dev, if speaker has specify one. When speaker is closed, this server stream shall be removed.
(4) Modify the logic when APM attaches to speaker's pipeline. If there exists echo reference dev for the speaker, APM  should attach to this echo ref dev instead.
 
Cc: dgreid@chromium.org
CLs uploaded for review:

https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/1180782 CRAS: alsa_ucm - label to speicfy echo reference iodev        
https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/1180783 CRAS: iodev_list - open server stream to activate echo reference        

Note:
 - The final tuned audio config files at https://crrev.com/c/1180786 
 - these changes have been verified on nocturne board.
 - should be the last piece of CRAS work to get fully tuned AEC on nocturne.

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/10d579e5297cf107beec9955d0947259305067d8

commit 10d579e5297cf107beec9955d0947259305067d8
Author: Hsin-Yu Chao <hychao@google.com>
Date: Sat Sep 15 00:05:42 2018

CRAS: iodev_list - open server stream to activate echo reference

To utilize the echo reference input device from driver. Add
the capability to create input stream without client. This helps
to establish the data flow from hardware to iodev's dsp pipeline
for APM reverse analysis.

The implementation requires change in several modules, including:

  - Add stream type to mark stream as server only, so it doesn't
    need to talk to client.
  - Reserve a few numbers for the "client id" composing the stream
    id of server only stream.
  - Flexibility for input cras_iodev to set different ext_dsp_mod
    to its dsp pipeline.
  - Add module server_stream to create/destroy server only stream
    pinned to specific device.
  - Logic in iodev_list to add/rm server only stream at appropriate
    time if echo reference dev exists.
  - Logic in apm_list to attach rmod to the dsp pipeline of echo
    reference iodev instead of the active output iodev.

BUG= chromium:879967 
TEST=Test APM using echo reference on nocturne board

Change-Id: I515cb3a85214e72d3378adb1a21ee9b4639420c8
Reviewed-on: https://chromium-review.googlesource.com/1180783
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/cras_rstream.c
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/tests/iodev_list_unittest.cc
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/common/cras_types.h
[add] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/server_stream.c
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/cras_apm_list.c
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/cras_rstream.h
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/tests/apm_list_unittest.cc
[add] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/server_stream.h
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/cras_server.h
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/Makefile.am
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/input_data.c
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/cras_server.c
[modify] https://crrev.com/10d579e5297cf107beec9955d0947259305067d8/cras/src/server/cras_iodev.c

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/536c28ee5ee1653cc5535b21d6a31435ed3f7f2b

commit 536c28ee5ee1653cc5535b21d6a31435ed3f7f2b
Author: Hsin-Yu Chao <hychao@google.com>
Date: Sat Sep 15 00:05:42 2018

CRAS: system_state - Add task support

Add the support for scheduling a function to be executed in main
thread loop later with no wait time. This is useful for scheduling
asynchronously tasks in main thread.

BUG= chromium:879967 
TEST=unittest
Apply patch set to schedule server stream for echo ref dev
and verify the function works as expected.

Change-Id: I71ceade96dc86b353ba14e0507a69e68f9e20082
Reviewed-on: https://chromium-review.googlesource.com/1212623
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/536c28ee5ee1653cc5535b21d6a31435ed3f7f2b/cras/src/server/cras_system_state.h
[modify] https://crrev.com/536c28ee5ee1653cc5535b21d6a31435ed3f7f2b/cras/src/server/cras_system_state.c
[modify] https://crrev.com/536c28ee5ee1653cc5535b21d6a31435ed3f7f2b/cras/src/tests/system_state_unittest.cc
[modify] https://crrev.com/536c28ee5ee1653cc5535b21d6a31435ed3f7f2b/cras/src/server/cras_server.c

Blockedon: 884893
Two issues are caused by my CL in #2

https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/1226926
https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/1229795

we should wait for them all land tot then merge to m70
Cc: geohsu@chromium.org
Labels: Merge-Request-70
CLs all landed and verified. Request merge to M70.
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 21

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-70 Merge-Approved-70
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 25

Labels: merge-merged-release-R70-11021.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/bac1956f0c05137f4356886e1faaeed3eaba41e0

commit bac1956f0c05137f4356886e1faaeed3eaba41e0
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Tue Sep 25 02:39:49 2018

CRAS: alsa_ucm - label to specify echo reference iodev

Add EchoReferenceDev label to associate one output device
to another input device as echo reference.

BUG= chromium:879967 
TEST=Test APM using echo reference on notcurne

Change-Id: I3394650549e6a9212bf95b80fc2d0a68e7286733
Reviewed-on: https://chromium-review.googlesource.com/1180782
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
(cherry picked from commit a3509d54ab348eedddeca2aa3653dc3ac0afebcb)
Reviewed-on: https://chromium-review.googlesource.com/1242494
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/bac1956f0c05137f4356886e1faaeed3eaba41e0/cras/src/tests/alsa_card_unittest.cc
[modify] https://crrev.com/bac1956f0c05137f4356886e1faaeed3eaba41e0/cras/src/tests/alsa_ucm_unittest.cc
[modify] https://crrev.com/bac1956f0c05137f4356886e1faaeed3eaba41e0/cras/src/server/cras_iodev.h
[modify] https://crrev.com/bac1956f0c05137f4356886e1faaeed3eaba41e0/cras/src/server/cras_alsa_ucm.c
[modify] https://crrev.com/bac1956f0c05137f4356886e1faaeed3eaba41e0/cras/src/server/cras_alsa_card.c
[modify] https://crrev.com/bac1956f0c05137f4356886e1faaeed3eaba41e0/cras/src/server/cras_alsa_ucm.h

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/aa557d1069c7da8a5b7ca6fc1e650e5aa4b3ee5a

commit aa557d1069c7da8a5b7ca6fc1e650e5aa4b3ee5a
Author: Hsin-Yu Chao <hychao@google.com>
Date: Tue Sep 25 02:43:33 2018

CRAS: system_state - Add task support

Add the support for scheduling a function to be executed in main
thread loop later with no wait time. This is useful for scheduling
asynchronously tasks in main thread.

BUG= chromium:879967 
TEST=unittest
Apply patch set to schedule server stream for echo ref dev
and verify the function works as expected.

Change-Id: I71ceade96dc86b353ba14e0507a69e68f9e20082
Reviewed-on: https://chromium-review.googlesource.com/1212623
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 536c28ee5ee1653cc5535b21d6a31435ed3f7f2b)
Reviewed-on: https://chromium-review.googlesource.com/1242495
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/aa557d1069c7da8a5b7ca6fc1e650e5aa4b3ee5a/cras/src/server/cras_system_state.h
[modify] https://crrev.com/aa557d1069c7da8a5b7ca6fc1e650e5aa4b3ee5a/cras/src/server/cras_system_state.c
[modify] https://crrev.com/aa557d1069c7da8a5b7ca6fc1e650e5aa4b3ee5a/cras/src/tests/system_state_unittest.cc
[modify] https://crrev.com/aa557d1069c7da8a5b7ca6fc1e650e5aa4b3ee5a/cras/src/server/cras_server.c

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/04da28020fd07615fc6546b8486253fea5147cba

commit 04da28020fd07615fc6546b8486253fea5147cba
Author: Hsin-Yu Chao <hychao@google.com>
Date: Tue Sep 25 02:47:07 2018

CRAS: iodev_list - open server stream to activate echo reference

To utilize the echo reference input device from driver. Add
the capability to create input stream without client. This helps
to establish the data flow from hardware to iodev's dsp pipeline
for APM reverse analysis.

The implementation requires change in several modules, including:

  - Add stream type to mark stream as server only, so it doesn't
    need to talk to client.
  - Reserve a few numbers for the "client id" composing the stream
    id of server only stream.
  - Flexibility for input cras_iodev to set different ext_dsp_mod
    to its dsp pipeline.
  - Add module server_stream to create/destroy server only stream
    pinned to specific device.
  - Logic in iodev_list to add/rm server only stream at appropriate
    time if echo reference dev exists.
  - Logic in apm_list to attach rmod to the dsp pipeline of echo
    reference iodev instead of the active output iodev.

BUG= chromium:879967 
TEST=Test APM using echo reference on nocturne board

Change-Id: I515cb3a85214e72d3378adb1a21ee9b4639420c8
Reviewed-on: https://chromium-review.googlesource.com/1180783
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 10d579e5297cf107beec9955d0947259305067d8)
Reviewed-on: https://chromium-review.googlesource.com/1242496
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/cras_rstream.c
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/tests/iodev_list_unittest.cc
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/common/cras_types.h
[add] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/server_stream.c
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/cras_apm_list.c
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/cras_rstream.h
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/tests/apm_list_unittest.cc
[add] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/server_stream.h
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/cras_server.h
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/Makefile.am
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/input_data.c
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/cras_server.c
[modify] https://crrev.com/04da28020fd07615fc6546b8486253fea5147cba/cras/src/server/cras_iodev.c

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/de6ce61acd8d54d87c5a451c43f6a68495bae6d5

commit de6ce61acd8d54d87c5a451c43f6a68495bae6d5
Author: Dylan Reid <dgreid@chromium.org>
Date: Tue Sep 25 02:53:58 2018

CRAS: alsa_card - handle null ucm in echo ref path

The card's ucm pointer might be null, if it is, then skip the echo
reference check as there can't be one.

BUG= chromium:879967 
TEST=None

Change-Id: I5eb4a3dd51d15ae6d3ce95a43cfa310ac5e1ae37
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1226926
(cherry picked from commit e6845fa74aa89b724b488d924c0388250ea1341b)
Reviewed-on: https://chromium-review.googlesource.com/1242497
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/de6ce61acd8d54d87c5a451c43f6a68495bae6d5/cras/src/server/cras_alsa_card.c

Labels: -Merge-Approved-70
Status: Fixed (was: Started)
Done merge to M70.

Sign in to add a comment