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

Issue 634958 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature



Sign in to add a comment

CRAS: Add ability to specify a volume curve for line-out headset jack connects.

Project Member Reported by dgreid@chromium.org, Aug 5 2016

Issue description

Now that some devices support line-out detection in addition to headphones, add the ability to specify a different volume curve for those devices.
 
Status: Started (was: Assigned)
With current implementation, volume curve uses mixer control of the node as key, like "Speaker" or "Headphone".
In our case the line-out and headphone jacks are both associated with the same mixer control and jack name(named by corresponding input device). In order for line-out to have its own volume curve, we'll modify code to look up volume curve using UCM device as well.


Maybe better to let the UCM specify a volume curve?

Device Headphones {
....
VolumeCurve {
 100 => ...dB
}


What do you think?  Too much UI specific stuff in UCM or would that work?
I would +1 to Hsinyu's approach. Some concerns include:
1. It will be much easier for partner to directly apply a volume curve file generated from tuning site rather than modifying HiFi.conf.
2. We have support for different curves for the same board using different device_config_dir.
3. Using UCM device name to find volume curve also fit in the logic of fully specified UCM although the curve itself is not in UCM.
Thanks!

Moving volume curve to UCM introduces another code path: parse, create curve, and associate to node, which is parallel to the existing card config & mixer path. It's additional effort to test and maintain both(otherwise we have to migrate all existing vol curve config to UCM)
After traced related code I think it's a good chance to clean up volume curve code to make the mapping more intuitive and flexible.  I have a few patches almost ready for that, will upload for review after more testing.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 11 2016

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

commit 34dfe567ba42b42b4ec701abe24187715aad8092
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Tue Aug 09 09:27:48 2016

CRAS: alsa_mixer - Allow control and jack to have NULL volume curve

Originally we create default volume curve for every output control
and jack if card config doesn't specify their own volume curve. This
makes it difficult to decide which curve to use for given active
output node in more complicated case.
This change allows mixer control and jack to have NULL volume curve
while not changing the behavior because card mixer always has a
non-NULL volume curve as fallback option.

BUG= chromium:634958 
TEST=Output from speaker and headphone, adjust output volume.

Change-Id: I4706afb77bab01e9f0cc88597c748da1b928aabf
Reviewed-on: https://chromium-review.googlesource.com/367680
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/34dfe567ba42b42b4ec701abe24187715aad8092/cras/src/server/config/cras_card_config.c
[modify] https://crrev.com/34dfe567ba42b42b4ec701abe24187715aad8092/cras/src/tests/alsa_io_unittest.cc
[modify] https://crrev.com/34dfe567ba42b42b4ec701abe24187715aad8092/cras/src/server/config/cras_card_config.h
[modify] https://crrev.com/34dfe567ba42b42b4ec701abe24187715aad8092/cras/src/tests/alsa_mixer_unittest.cc
[modify] https://crrev.com/34dfe567ba42b42b4ec701abe24187715aad8092/cras/src/server/cras_alsa_mixer.h
[modify] https://crrev.com/34dfe567ba42b42b4ec701abe24187715aad8092/cras/src/server/cras_alsa_mixer.c
[modify] https://crrev.com/34dfe567ba42b42b4ec701abe24187715aad8092/cras/src/tests/card_config_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 11 2016

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

commit e99b3f402531a36baaba0a315421c8b09af3e5b6
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Tue Aug 09 08:29:23 2016

CRAS: alsa_io - Look up volume curve by jack first

Changes the look up order for which volume curve to use on an active
output node. This allows codec which supports headphone and line-out
jacks to use different volume curve while sharing the same mixer
control.

BUG= chromium:634958 
TEST=

Change-Id: Ia3bc713a94f3fbd776e479aee82cf1b70ed346bc
Reviewed-on: https://chromium-review.googlesource.com/367681
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/e99b3f402531a36baaba0a315421c8b09af3e5b6/cras/src/tests/alsa_io_unittest.cc
[modify] https://crrev.com/e99b3f402531a36baaba0a315421c8b09af3e5b6/cras/src/server/cras_alsa_io.c

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 11 2016

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

commit 9fb7a508313ec89b837ccc66ddb7b24648b6666a
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Tue Aug 09 09:15:52 2016

CRAS: alsa_io - Get volume curve for node by UCM

This changes allows spcifying volume curve for a node associated
to specific UCM device, which is useful in the case that 'Line Out'
and 'Headphone' nodes shares the same gpio jack and mixer control.

BUG= chromium:634958 
TEST=Edit CRAS config under /etc/cras and specify the section label
as the UCM device of a node to verify volume curve applied.

Change-Id: Icec8de79385423922b9a0d52942cee833c8a07cc
Reviewed-on: https://chromium-review.googlesource.com/367682
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/9fb7a508313ec89b837ccc66ddb7b24648b6666a/cras/src/server/cras_alsa_jack.c
[modify] https://crrev.com/9fb7a508313ec89b837ccc66ddb7b24648b6666a/cras/src/server/cras_alsa_jack.h
[modify] https://crrev.com/9fb7a508313ec89b837ccc66ddb7b24648b6666a/cras/src/tests/alsa_io_unittest.cc
[modify] https://crrev.com/9fb7a508313ec89b837ccc66ddb7b24648b6666a/cras/src/server/cras_alsa_io.c

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 11 2016

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

commit 31ad060013574bbf3cec5e20c63e6b3c0986ab1b
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Wed Aug 10 12:59:21 2016

CRAS: README - Explain the rule to match node in card config

BUG= chromium:634958 
TEST=None

Change-Id: Ied424470e7285354cc78a15bb4b3fd1a910d1cdd
Reviewed-on: https://chromium-review.googlesource.com/367683
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/31ad060013574bbf3cec5e20c63e6b3c0986ab1b/cras/README

Comment 9 by hychao@chromium.org, Aug 12 2016

Cc: keta...@chromium.org
Labels: -M-54 Merge-Request-53 M-53
We have a board might need this in M53.

Comment 10 by dimu@chromium.org, Aug 12 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 11 by sheriffbot@chromium.org, Aug 16 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -M-53 -Hotlist-Merge-Approved -Merge-Approved-53 M-54
Status: Fixed (was: Started)
turns out it aims for M54.
Hi Hsinyu, can you please instruct when and how should we verify this fix. Thanks a lot!  

Comment 14 by ka...@chromium.org, Oct 14 2016

Status: Verified (was: Fixed)

Sign in to add a comment