Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(117)

Issue 2771143002: Implement runtime audio post-processing pipeline. See go/cast_audio.json (Closed)

Created:
3 years, 9 months ago by bshaya
Modified:
3 years, 9 months ago
Reviewers:
slan, bcf, wzhong, gfhuang, jyw, kmackay
CC:
chromium-reviews, alokp+watch_chromium.org, lcwu+watch_chromium.org, gfhuang+watch_chromium.org, halliwell+watch_chromium.org, feature-media-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement runtime audio post-processing pipeline. See go/cast_audio.json Bug=internal/36299959 Test=On Device. Change-Id: Ie94f7fd798d30016e35303c2f44ba186b8d47336 Review-Url: https://codereview.chromium.org/2771143002 Cr-Commit-Position: refs/heads/master@{#460003} Committed: https://chromium.googlesource.com/chromium/src/+/79101517b16f19a4cba75612bf40813f1593f315

Patch Set 1 #

Patch Set 2 : Fix stray keystroke #

Total comments: 4

Patch Set 3 : Fix clang compile issues. #

Patch Set 4 : Remove explicit dlsym include. #

Total comments: 2

Patch Set 5 : Remove unordered_map of libraries. #

Total comments: 6

Patch Set 6 : Address CR comments. #

Total comments: 8

Patch Set 7 : Fix nits #

Patch Set 8 : Fix compilation #

Total comments: 2

Patch Set 9 : Suffix governor with _1.0 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+553 lines, -170 lines) Patch
M chromecast/media/cma/backend/alsa/BUILD.gn View 1 2 3 4 5 6 7 8 5 chunks +18 lines, -30 lines 0 comments Download
D chromecast/media/cma/backend/alsa/audio_filter_factory.h View 1 chunk +0 lines, -33 lines 0 comments Download
D chromecast/media/cma/backend/alsa/audio_filter_factory_default.cc View 1 chunk +0 lines, -18 lines 0 comments Download
D chromecast/media/cma/backend/alsa/audio_filter_interface.h View 1 chunk +0 lines, -28 lines 0 comments Download
M chromecast/media/cma/backend/alsa/filter_group.h View 1 2 5 chunks +11 lines, -8 lines 0 comments Download
M chromecast/media/cma/backend/alsa/filter_group.cc View 1 2 3 4 5 6 5 chunks +25 lines, -44 lines 0 comments Download
A chromecast/media/cma/backend/alsa/post_processing_pipeline.h View 1 2 3 4 5 6 7 1 chunk +60 lines, -0 lines 0 comments Download
A chromecast/media/cma/backend/alsa/post_processing_pipeline.cc View 1 2 3 4 1 chunk +113 lines, -0 lines 0 comments Download
A chromecast/media/cma/backend/alsa/post_processing_pipeline_parser.h View 1 2 3 4 5 6 7 1 chunk +52 lines, -0 lines 0 comments Download
A chromecast/media/cma/backend/alsa/post_processing_pipeline_parser.cc View 1 2 3 4 5 6 7 1 chunk +83 lines, -0 lines 0 comments Download
A chromecast/media/cma/backend/alsa/post_processors/governor_shlib.cc View 1 2 3 4 5 6 1 chunk +106 lines, -0 lines 0 comments Download
M chromecast/media/cma/backend/alsa/stream_mixer_alsa.h View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M chromecast/media/cma/backend/alsa/stream_mixer_alsa.cc View 1 2 3 4 5 4 chunks +18 lines, -8 lines 0 comments Download
M chromecast/public/media/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
A chromecast/public/media/audio_post_processor_shlib.h View 1 2 1 chunk +66 lines, -0 lines 0 comments Download

Messages

Total messages: 30 (13 generated)
bshaya
3 years, 9 months ago (2017-03-24 03:18:41 UTC) #2
gfhuang
On 2017/03/24 03:18:41, bshaya wrote: lgtm. thanks!
3 years, 9 months ago (2017-03-24 03:35:07 UTC) #3
bcf
lgtm
3 years, 9 months ago (2017-03-24 04:23:36 UTC) #4
kmackay
https://codereview.chromium.org/2771143002/diff/20001/chromecast/media/cma/backend/alsa/post_processing_pipeline.cc File chromecast/media/cma/backend/alsa/post_processing_pipeline.cc (right): https://codereview.chromium.org/2771143002/diff/20001/chromecast/media/cma/backend/alsa/post_processing_pipeline.cc#newcode65 chromecast/media/cma/backend/alsa/post_processing_pipeline.cc:65: CHECK(!(error = dlerror())) << "Could not find " << ...
3 years, 9 months ago (2017-03-24 04:47:58 UTC) #5
bshaya
https://codereview.chromium.org/2771143002/diff/20001/chromecast/media/cma/backend/alsa/post_processing_pipeline.cc File chromecast/media/cma/backend/alsa/post_processing_pipeline.cc (right): https://codereview.chromium.org/2771143002/diff/20001/chromecast/media/cma/backend/alsa/post_processing_pipeline.cc#newcode65 chromecast/media/cma/backend/alsa/post_processing_pipeline.cc:65: CHECK(!(error = dlerror())) << "Could not find " << ...
3 years, 9 months ago (2017-03-24 18:55:10 UTC) #6
wzhong
https://codereview.chromium.org/2771143002/diff/60001/chromecast/media/cma/backend/alsa/post_processing_pipeline.h File chromecast/media/cma/backend/alsa/post_processing_pipeline.h (right): https://codereview.chromium.org/2771143002/diff/60001/chromecast/media/cma/backend/alsa/post_processing_pipeline.h#newcode53 chromecast/media/cma/backend/alsa/post_processing_pipeline.h:53: std::unordered_map<std::string, std::unique_ptr<base::ScopedNativeLibrary>> map is not necessary as I've commented ...
3 years, 9 months ago (2017-03-24 19:22:29 UTC) #7
bshaya
https://codereview.chromium.org/2771143002/diff/60001/chromecast/media/cma/backend/alsa/post_processing_pipeline.h File chromecast/media/cma/backend/alsa/post_processing_pipeline.h (right): https://codereview.chromium.org/2771143002/diff/60001/chromecast/media/cma/backend/alsa/post_processing_pipeline.h#newcode53 chromecast/media/cma/backend/alsa/post_processing_pipeline.h:53: std::unordered_map<std::string, std::unique_ptr<base::ScopedNativeLibrary>> On 2017/03/24 19:22:29, wzhong wrote: > map ...
3 years, 9 months ago (2017-03-24 21:32:25 UTC) #8
kmackay
https://codereview.chromium.org/2771143002/diff/80001/chromecast/media/cma/backend/alsa/BUILD.gn File chromecast/media/cma/backend/alsa/BUILD.gn (left): https://codereview.chromium.org/2771143002/diff/80001/chromecast/media/cma/backend/alsa/BUILD.gn#oldcode109 chromecast/media/cma/backend/alsa/BUILD.gn:109: "//media", //media is still needed for eg media/audio/audio_device_description.h https://codereview.chromium.org/2771143002/diff/80001/chromecast/media/cma/backend/alsa/filter_group.cc ...
3 years, 9 months ago (2017-03-24 22:15:54 UTC) #11
bshaya
https://codereview.chromium.org/2771143002/diff/80001/chromecast/media/cma/backend/alsa/BUILD.gn File chromecast/media/cma/backend/alsa/BUILD.gn (left): https://codereview.chromium.org/2771143002/diff/80001/chromecast/media/cma/backend/alsa/BUILD.gn#oldcode109 chromecast/media/cma/backend/alsa/BUILD.gn:109: "//media", On 2017/03/24 22:15:54, kmackay wrote: > //media is ...
3 years, 9 months ago (2017-03-24 22:55:49 UTC) #12
kmackay
lgtm
3 years, 9 months ago (2017-03-25 03:19:46 UTC) #13
wzhong
lgtm Some nits. https://codereview.chromium.org/2771143002/diff/100001/chromecast/media/cma/backend/alsa/filter_group.h File chromecast/media/cma/backend/alsa/filter_group.h (right): https://codereview.chromium.org/2771143002/diff/100001/chromecast/media/cma/backend/alsa/filter_group.h#newcode16 chromecast/media/cma/backend/alsa/filter_group.h:16: #include "base/values.h" Nit: forward declaration. https://codereview.chromium.org/2771143002/diff/100001/chromecast/media/cma/backend/alsa/post_processing_pipeline_parser.h ...
3 years, 9 months ago (2017-03-27 14:23:43 UTC) #14
bshaya
https://codereview.chromium.org/2771143002/diff/100001/chromecast/media/cma/backend/alsa/filter_group.h File chromecast/media/cma/backend/alsa/filter_group.h (right): https://codereview.chromium.org/2771143002/diff/100001/chromecast/media/cma/backend/alsa/filter_group.h#newcode16 chromecast/media/cma/backend/alsa/filter_group.h:16: #include "base/values.h" On 2017/03/27 14:23:43, wzhong wrote: > Nit: ...
3 years, 9 months ago (2017-03-27 21:00:26 UTC) #15
slan
lgtm for OWNERS stamp, deferring to others' review. https://codereview.chromium.org/2771143002/diff/140001/chromecast/media/cma/backend/alsa/BUILD.gn File chromecast/media/cma/backend/alsa/BUILD.gn (right): https://codereview.chromium.org/2771143002/diff/140001/chromecast/media/cma/backend/alsa/BUILD.gn#newcode135 chromecast/media/cma/backend/alsa/BUILD.gn:135: shared_library("libcast_governor") ...
3 years, 9 months ago (2017-03-28 00:32:28 UTC) #16
bshaya
https://codereview.chromium.org/2771143002/diff/140001/chromecast/media/cma/backend/alsa/BUILD.gn File chromecast/media/cma/backend/alsa/BUILD.gn (right): https://codereview.chromium.org/2771143002/diff/140001/chromecast/media/cma/backend/alsa/BUILD.gn#newcode135 chromecast/media/cma/backend/alsa/BUILD.gn:135: shared_library("libcast_governor") { On 2017/03/28 00:32:28, slan wrote: > Is ...
3 years, 9 months ago (2017-03-28 02:18:05 UTC) #17
bshaya
3 years, 9 months ago (2017-03-28 02:53:30 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2771143002/160001
3 years, 9 months ago (2017-03-28 03:34:20 UTC) #27
commit-bot: I haz the power
3 years, 9 months ago (2017-03-28 03:43:07 UTC) #30
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/79101517b16f19a4cba75612bf40...

Powered by Google App Engine
This is Rietveld 408576698