Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1551613002:
Initialize input AUHAL before using it (Closed)
|
Created:
4 years, 12 months ago by henrika (OOO until Aug 14) Modified:
4 years, 12 months ago Reviewers:
tommi (sloooow) - chröme CC:
chromium-reviews, feature-media-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionInitialize input AUHAL before using it.
This CL is an attempt to ensure that input audio recording always works on Mac OS X.
It is a trivial change and I don't know if it has any real effect, hence it is speculative.
I am making this change since I have seen in examples that it is recommended to initialize
the AUHAL before using it. We can only hope that it has a positive effect.
More small changes are in the pipeline but my plan was to land them one by one to keep
the changes as simple as possible and easy to revert if needed.
Adding tommi@ as TBR since he is OOO and the change can be considered trivial. If we are lucky
it has a positive effect. If we are not, I don't expect any effect at all.
TBR=tommi
BUG=549021
Committed: https://crrev.com/686d21113c2362ecf7e75c8d88a47450bc9485a6
Cr-Commit-Position: refs/heads/master@{#366977}
Patch Set 1 #
Total comments: 4
Messages
Total messages: 14 (8 generated)
Description was changed from ========== Initialize input AUHAL before using it BUG= ========== to ========== Initialize input AUHAL before using it. BUG=549021 ==========
Description was changed from ========== Initialize input AUHAL before using it. BUG=549021 ========== to ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. TBR=tommi BUG=549021 ==========
Description was changed from ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. TBR=tommi BUG=549021 ========== to ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. Adding tommi@ as TBR since he is OOO and the change can be considered trivial. If we are lucky it has a positive effect. If we are not, I don't expect any effect at all. TBR=tommi BUG=549021 ==========
henrika@chromium.org changed reviewers: + tommi@chromium.org
Description was changed from ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. Adding tommi@ as TBR since he is OOO and the change can be considered trivial. If we are lucky it has a positive effect. If we are not, I don't expect any effect at all. TBR=tommi BUG=549021 ========== to ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. More small changes are in the pipeline but my plan was to land them one by one to keep the changes as simple as possible and easy to revert if needed. Adding tommi@ as TBR since he is OOO and the change can be considered trivial. If we are lucky it has a positive effect. If we are not, I don't expect any effect at all. TBR=tommi BUG=549021 ==========
The CQ bit was checked by henrika@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1551613002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1551613002/1
Message was sent while issue was closed.
Description was changed from ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. More small changes are in the pipeline but my plan was to land them one by one to keep the changes as simple as possible and easy to revert if needed. Adding tommi@ as TBR since he is OOO and the change can be considered trivial. If we are lucky it has a positive effect. If we are not, I don't expect any effect at all. TBR=tommi BUG=549021 ========== to ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. More small changes are in the pipeline but my plan was to land them one by one to keep the changes as simple as possible and easy to revert if needed. Adding tommi@ as TBR since he is OOO and the change can be considered trivial. If we are lucky it has a positive effect. If we are not, I don't expect any effect at all. TBR=tommi BUG=549021 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. More small changes are in the pipeline but my plan was to land them one by one to keep the changes as simple as possible and easy to revert if needed. Adding tommi@ as TBR since he is OOO and the change can be considered trivial. If we are lucky it has a positive effect. If we are not, I don't expect any effect at all. TBR=tommi BUG=549021 ========== to ========== Initialize input AUHAL before using it. This CL is an attempt to ensure that input audio recording always works on Mac OS X. It is a trivial change and I don't know if it has any real effect, hence it is speculative. I am making this change since I have seen in examples that it is recommended to initialize the AUHAL before using it. We can only hope that it has a positive effect. More small changes are in the pipeline but my plan was to land them one by one to keep the changes as simple as possible and easy to revert if needed. Adding tommi@ as TBR since he is OOO and the change can be considered trivial. If we are lucky it has a positive effect. If we are not, I don't expect any effect at all. TBR=tommi BUG=549021 Committed: https://crrev.com/686d21113c2362ecf7e75c8d88a47450bc9485a6 Cr-Commit-Position: refs/heads/master@{#366977} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/686d21113c2362ecf7e75c8d88a47450bc9485a6 Cr-Commit-Position: refs/heads/master@{#366977}
Message was sent while issue was closed.
https://codereview.chromium.org/1551613002/diff/1/media/audio/mac/audio_low_l... File media/audio/mac/audio_low_latency_input_mac.cc (right): https://codereview.chromium.org/1551613002/diff/1/media/audio/mac/audio_low_l... media/audio/mac/audio_low_latency_input_mac.cc:138: // The user specifies which audio device to track. The audio unit can do Nit:spacing https://codereview.chromium.org/1551613002/diff/1/media/audio/mac/audio_low_l... media/audio/mac/audio_low_latency_input_mac.cc:249: if (result != noErr) { Remove this call then? Can it fail now that initialize has already been called?
Message was sent while issue was closed.
Thanks for checking even on your vacation Tommi. Hope we can give this change a couple of days and then see if it has any positive effect. https://codereview.chromium.org/1551613002/diff/1/media/audio/mac/audio_low_l... File media/audio/mac/audio_low_latency_input_mac.cc (right): https://codereview.chromium.org/1551613002/diff/1/media/audio/mac/audio_low_l... media/audio/mac/audio_low_latency_input_mac.cc:138: // The user specifies which audio device to track. The audio unit can do My bad; hope it is OK if I skip uploading a fix for it now but instead include it in my next attempt to fix things in this area. https://codereview.chromium.org/1551613002/diff/1/media/audio/mac/audio_low_l... media/audio/mac/audio_low_latency_input_mac.cc:249: if (result != noErr) { No, that is the trick actually, to do it twice (and it does not fail the second time). The first call is (I guess) more done as some sort of reset-before-making-any-changes, or "flush" operation. I have seen it in a widely spread (and maintained) Apple example and the exact comment is "AUHAL needs to be initialized before anything is done to it". I can't find a verification in any official API doc that it *needs* to be pre-initialized but the fact that they say *needs* shows that it is worth a try. Hope you agree.
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/1573503002/ by henrika@chromium.org. The reason for reverting is: I am reverting a set of dependent changes since I have found out that the existing code breaks the WebSpeech API. I will take a few steps back and restore this code but at the same time ensure that I don't break the WebSpeech API. Hence, this fix will be relanded but in a slightly different format probably.. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
