Issue metadata
Sign in to add a comment
|
A/V Sync problem with AirPlay
Reported by
davidsar...@gmail.com,
Nov 9 2017
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36 Example URL: mlbtv.com Steps to reproduce the problem: 1. Route Audio on your Mac to an AirPort Express or other AirPlay Audio device (not AppleTV) using the Audio Picker in the Menu Bar (set the Output Device) 2. Got watch a video on mlbtv.com or YouTube What is the expected behavior? Audio and Video are synchronized What went wrong? Audio is 2 seconds behind the Video. It looks like the output latency is not being accounted for. Did this work before? N/A Is it a problem with Flash or HTML5? N/A Does this work in other browsers? Yes Chrome version: 61.0.3163.100 Channel: n/a OS Version: OS X 10.13.4 Flash Version: Contents of chrome://gpu:
,
Nov 9 2017
,
Nov 11 2017
For posterity, via e-mail it was pointed out that we're not taking into account the stream latency per this note in the header documentation:
@constant kAudioDevicePropertyLatency
A UInt32 containing the number of frames of latency in the AudioDevice. Note
that input and output latency may differ. Further, the AudioDevice's
AudioStreams may have additional latency so they should be queried as well.
If both the device and the stream say they have latency, then the total
latency for the stream is the device latency summed with the stream latency.
To wit, WebRTCs own internal driver already does this:
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/audio_device/mac/audio_device_mac.cc?l=1267
,
Nov 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/973509d48434c287fce4510b3a2fe218285a50c4 commit 973509d48434c287fce4510b3a2fe218285a50c4 Author: Dale Curtis <dalecurtis@chromium.org> Date: Mon Nov 13 21:24:50 2017 Include stream latency for macOS audio latency calculations. We were incorrectly ignoring the stream latency component when calculating the fixed latency of streams. This needs to be done for both input and output. Since these methods are very similar I've refactored the one our input and output streams had into a common AudioMangerMac method. BUG= 783282 TEST=none Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: Ic583a2f314728b566a5a3e3de8c8ffd118652f91 Reviewed-on: https://chromium-review.googlesource.com/764542 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Olga Sharonova <olka@chromium.org> Cr-Commit-Position: refs/heads/master@{#516056} [modify] https://crrev.com/973509d48434c287fce4510b3a2fe218285a50c4/media/audio/mac/audio_auhal_mac.cc [modify] https://crrev.com/973509d48434c287fce4510b3a2fe218285a50c4/media/audio/mac/audio_auhal_mac.h [modify] https://crrev.com/973509d48434c287fce4510b3a2fe218285a50c4/media/audio/mac/audio_low_latency_input_mac.cc [modify] https://crrev.com/973509d48434c287fce4510b3a2fe218285a50c4/media/audio/mac/audio_low_latency_input_mac.h [modify] https://crrev.com/973509d48434c287fce4510b3a2fe218285a50c4/media/audio/mac/audio_manager_mac.cc [modify] https://crrev.com/973509d48434c287fce4510b3a2fe218285a50c4/media/audio/mac/audio_manager_mac.h
,
Nov 29 2017
Marking Fixed, please ping if it's still an issue.
,
Nov 29 2017
Still occurs with Chrome Canary v 64.0.3279.0. Is there a way to log the latency being used by the audio pipeline? Thanks.
,
Nov 29 2017
No, but we should probably have one. Will send a CL.
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eba82e8dfc3a9f43cb740a408959d23301d158f5 commit eba82e8dfc3a9f43cb740a408959d23301d158f5 Author: Dale Curtis <dalecurtis@chromium.org> Date: Wed Nov 29 02:15:47 2017 Add trace information for audio output delay data. Useful in debugging delay issues. BUG= 783282 TEST=chrome://tracing has delay information. Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I4973c451efa2236d680bbef334c2a49d073a7c81 Reviewed-on: https://chromium-review.googlesource.com/794972 Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#519968} [modify] https://crrev.com/eba82e8dfc3a9f43cb740a408959d23301d158f5/media/audio/audio_output_device.cc
,
Dec 1 2017
For posterity, via e-mail the advice from Apple was, "You should just add latency of a single stream and add that to the device latency. In your case, I’d take the stream at index zero. This will fail when there’s a device with multiple streams each having different latencies - think a break-out box with both a S/PDIF and an analog out. But that’s pretty rare and the code isn’t really architected to handle that case anyway, so taking the zeroth index should get you there 99% of the time"
,
Dec 1 2017
(This also means the driver in WebRTC code base is incorrectly using device id)
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/93e742112a49d6ded6489f87bbf10b337b9b9f00 commit 93e742112a49d6ded6489f87bbf10b337b9b9f00 Author: Dale Curtis <dalecurtis@chromium.org> Date: Fri Dec 01 18:18:16 2017 Use stream id to get stream latency for first stream. The previous change 973509d48434c287fce4510b3a2fe218285a50c4, which added querying of stream latency, incorrectly uses the device id for querying the stream latency. We must instead, first query the list of stream ids and query the latency of a given stream. While there may be multiple streams, we can't do anything with that information; e.g., to handle that case we would need to attempt to synchronize all streams to the highest latency. Apple instead recommends just taking the latency of the first stream since that will cover the majority of cases. BUG= 783282 TEST=none Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I98b65b1fa7030f63abbb6d96c7540352d1bb896b Reviewed-on: https://chromium-review.googlesource.com/802194 Reviewed-by: Olga Sharonova <olka@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#520988} [modify] https://crrev.com/93e742112a49d6ded6489f87bbf10b337b9b9f00/media/audio/mac/audio_manager_mac.cc
,
Dec 4 2017
Fix verified by reporter.
,
Dec 4 2017
Will let it soak on the next dev for a week and request merge to m64 after.
,
Dec 11 2017
The NextAction date has arrived: 2017-12-11
,
Dec 11 2017
,
Dec 12 2017
Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b1a8da88794be9fba341adbe7537ea2e8646f732 commit b1a8da88794be9fba341adbe7537ea2e8646f732 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Dec 12 22:53:26 2017 Merge M64: "Use stream id to get stream latency for first stream." The previous change 973509d48434c287fce4510b3a2fe218285a50c4, which added querying of stream latency, incorrectly uses the device id for querying the stream latency. We must instead, first query the list of stream ids and query the latency of a given stream. While there may be multiple streams, we can't do anything with that information; e.g., to handle that case we would need to attempt to synchronize all streams to the highest latency. Apple instead recommends just taking the latency of the first stream since that will cover the majority of cases. BUG= 783282 TEST=none TBR=dalecurtis@chromium.org (cherry picked from commit 93e742112a49d6ded6489f87bbf10b337b9b9f00) Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I98b65b1fa7030f63abbb6d96c7540352d1bb896b Reviewed-on: https://chromium-review.googlesource.com/802194 Reviewed-by: Olga Sharonova <olka@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#520988} Reviewed-on: https://chromium-review.googlesource.com/823253 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/branch-heads/3282@{#185} Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840} [modify] https://crrev.com/b1a8da88794be9fba341adbe7537ea2e8646f732/media/audio/mac/audio_manager_mac.cc |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by davidsar...@gmail.com
, Nov 9 2017