Validate midstream audio config changes with CHANNEL_LAYOUT_DISCRETE [chromium/src : master]
Felicia Lim (Gerrit)
hmm, I thought I'd started the review but apparently not, so here's another try.
To view, visit change 972327. To unsubscribe, or for help writing mail filters, visit settings.
Dale Curtis (Gerrit)
lgtm in either case.
Patch set 1:Code-Review +1
2 comments:
File media/renderers/audio_renderer_impl.cc:
Patch Set #1, Line 683:
sink_->IsOptimizedForHardwareParameters()) {Is this extra check on the sink necessary? Or should we delete this and say that DISCRETE streams must start discrete and never downgrade? I.e. any channel layout change once DISCRETE has been set is not allowed.
Patch Set #1, Line 685:
<< "Unsupported midstream configuration change!""Unsupported midstream configuration change! Discrete channel layout not allowed by sink."
To view, visit change 972327. To unsubscribe, or for help writing mail filters, visit settings.
Felicia Lim (Gerrit)
Thanks for looking!
Patch set 2:Commit-Queue +2
2 comments:
File media/renderers/audio_renderer_impl.cc:
Patch Set #1, Line 683:
MEDIA_LOG(ERROR, media_log_)Is this extra check on the sink necessary? Or should we delete this and say that DISCRETE streams mu […]
Deleting this check makes sense, can't think of a good use case for allowing changes from discrete streams.
Patch Set #1, Line 685:
<< " layout not allowed by sink.";"Unsupported midstream configuration change! Discrete channel layout not allowed by sink. […]
Done
To view, visit change 972327. To unsubscribe, or for help writing mail filters, visit settings.
Commit Bot (Gerrit)
CQ is trying the patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"Remove extra check on sink and simplify comment" https://chromium-review.googlesource.com/c/972327/2
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/972327/2
Bot data: {"action": "start", "triggered_at": "2018-03-26T19:08:00.0Z", "cq_cfg_revision": "5b6c43e4d6b0297aa92e118e785d640c42297271", "revision": "7bbd82e30f493dfd84d4ef905bfe541b8e74fa9e"}
Commit Bot (Gerrit)
Try jobs failed on following builders:
ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?))
Felicia Lim (Gerrit)
Patch set 2:Commit-Queue +2
Commit Bot (Gerrit)
CQ is trying the patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"Remove extra check on sink and simplify comment" https://chromium-review.googlesource.com/c/972327/2
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/972327/2
Bot data: {"action": "start", "triggered_at": "2018-03-27T21:17:23.0Z", "cq_cfg_revision": "354223ef8a9beaefb02393573b81bd63fb2ab219", "revision": "7bbd82e30f493dfd84d4ef905bfe541b8e74fa9e"}
Commit Bot (Gerrit)
Try jobs failed on following builders:
mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/681188)
Felicia Lim (Gerrit)
Patch set 2:Commit-Queue +2
Commit Bot (Gerrit)
CQ is trying the patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"Remove extra check on sink and simplify comment" https://chromium-review.googlesource.com/c/972327/2
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/972327/2
Bot data: {"action": "start", "triggered_at": "2018-03-28T04:27:06.0Z", "cq_cfg_revision": "8ec12a5d6a407bc41085f8c1d9e56fa92b6add10", "revision": "7bbd82e30f493dfd84d4ef905bfe541b8e74fa9e"}
Commit Bot (Gerrit)
Commit Bot merged this change.
Validate midstream audio config changes with CHANNEL_LAYOUT_DISCRETE
Bug: 822744
Change-Id: I483e3f9644f4087295a0be95f9def4c6286ec72b
Reviewed-on: https://chromium-review.googlesource.com/972327
Commit-Queue: Felicia Lim <fl...@chromium.org>
Reviewed-by: Dale Curtis <dalec...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546403}
---
M media/renderers/audio_renderer_impl.cc
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/media/renderers/audio_renderer_impl.cc b/media/renderers/audio_renderer_impl.cc
index abcb3df..269d347 100644
--- a/media/renderers/audio_renderer_impl.cc
+++ b/media/renderers/audio_renderer_impl.cc
@@ -679,12 +679,17 @@
last_decoded_sample_rate_ = buffer->sample_rate();
if (last_decoded_channel_layout_ != buffer->channel_layout()) {
- last_decoded_channel_layout_ = buffer->channel_layout();
- last_decoded_channels_ = buffer->channel_count();
-
- // Input layouts should never be discrete.
- DCHECK_NE(last_decoded_channel_layout_, CHANNEL_LAYOUT_DISCRETE);
- ConfigureChannelMask();
+ if (buffer->channel_layout() == CHANNEL_LAYOUT_DISCRETE) {
+ MEDIA_LOG(ERROR, media_log_)
+ << "Unsupported midstream configuration change! Discrete channel"
+ << " layout not allowed by sink.";
+ HandleAbortedReadOrDecodeError(PIPELINE_ERROR_DECODE);
+ return;
+ } else {
+ last_decoded_channel_layout_ = buffer->channel_layout();
+ last_decoded_channels_ = buffer->channel_count();
+ ConfigureChannelMask();
+ }
}
}
To view, visit change 972327. To unsubscribe, or for help writing mail filters, visit settings.