Audio/video sync lost when hotswapping sound output

VERIFIED FIXED

Status

()

Core
Audio/Video
VERIFIED FIXED
9 years ago
7 years ago

People

(Reporter: kinetik, Assigned: kinetik)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(blocking2.0 -)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
This could be OS X only.  It's easy for me to reproduce, but might require two sound output devices, so I'll probably end up having to fix this myself.

0. Set external USB soundcard as default output device
1. Start video playing (bug URL is good for testing sync)
2. Unplug USB soundcard, OS automatically moves sound to default internal device
3. Video is now out of sync
(Assignee)

Comment 1

8 years ago
It's actually easy to reproduce this with a pair of headphones.  While a video is playing, unplug and replug headphones into the headphone jack of a Mac and A/V sync will break.

While the audio device is reconfiguring playback stops, but when it resumes the callback timestamp (which we use as the audio clock) has been adjusted for the pause period (so it's as if the clock continued ticking--it's probably based on the system clock).  When playback resumes, we notice that the audio clock is far ahead of the current frame time and drop a bunch of frames to catch up.  We write the audio attached to those frames as we drop them, but some of the audio should be dropped as it was intended to play during the time which we missed during audio reconfiguration.

There doesn't seem to be a way to detect these clock discontinuities, so other than having timestamps attached to the audio to decide when to play it, I'm not sure what to do.  I notice the newer (Leopard-only) AudioQueue API has support for detecting when clock discontinuities happen.
(Assignee)

Updated

8 years ago
Duplicate of this bug: 605944

Updated

8 years ago
blocking2.0: --- → ?
(Assignee)

Comment 3

8 years ago
I don't know how to fix this for 2.0, so I'd prefer it doesn't block.  It's on the list of things to fix post-2.0 as part of the Great Audio Rewrite.  If someone else knows how to fix it with the current sydneyaudio implementation, I have no problems taking patches.
(Assignee)

Comment 4

7 years ago
It's hard to find any discussion of this problem when searching for the API that we're using, but it turns out that there's a bunch of info if you look for people having the same problem with AudioDeviceGetCurrentTime.

So, digging around a bit... the bad news is it sounds like the newer API I am planning to use suffers from similar problems.

The good news is it might be possible to avoid this by using AUHAL, as that may have code to detect and correct time discontinuities.  That would involve changing kAudioUnitSubType_DefaultOutput to kAudioUnitSubType_HALOutput and possibly doing some additional setup.

Another possibility is setting up a kAudioDevicePropertyDeviceHasChanged (and possibly kAudioStreamPropertyPhysicalFormat) listener to detect device changes and do some kind of reset when this happens.
After discussions on irc last night, I tried the following change, and it didn't help.

diff --git a/media/libsydneyaudio/src/sydney_audio_mac.c b/media/libsydneyaudio/src/sydney_audio_mac.c
--- a/media/libsydneyaudio/src/sydney_audio_mac.c
+++ b/media/libsydneyaudio/src/sydney_audio_mac.c
@@ -184,7 +184,7 @@
    */
   ComponentDescription desc;
   desc.componentType         = kAudioUnitType_Output;
-  desc.componentSubType      = kAudioUnitSubType_DefaultOutput;
+  desc.componentSubType      = kAudioUnitSubType_HALOutput;
   desc.componentManufacturer = kAudioUnitManufacturer_Apple;
Created attachment 512040 [details] [diff] [review]
Yury's patch

Assuming that bug 633796 is the same as this, which I think is the case, the attached patch Yury wrote, and I just tested on Mac for him, fixes this.
Duplicate of this bug: 633796

Updated

7 years ago
Assignee: nobody → async.processingjs
(Assignee)

Comment 8

7 years ago
(In reply to comment #4)
> The good news is it might be possible to avoid this by using AUHAL, as that may
> have code to detect and correct time discontinuities.  That would involve
> changing kAudioUnitSubType_DefaultOutput to kAudioUnitSubType_HALOutput and
> possibly doing some additional setup.

This doesn't work. The behaviour is exactly the same as with DefaultOutput.

> Another possibility is setting up a kAudioDevicePropertyDeviceHasChanged (and
> possibly kAudioStreamPropertyPhysicalFormat) listener to detect device changes
> and do some kind of reset when this happens.

This could work for headphone/USB device changes, but I don't think it'll solve the sleep case from bug 633796.  It's easy to reproduce similar behaviour to bug 633796 by suspending Firefox in the terminal with C-z.

Yury's patch is the best route for now.  With the patch the times calculated are exactly the same as those based on mSampleTime when it has sane values, so there's no obvious disadvantage to taking this route.  Taking this approach, we can get rid of the time reset code in sa_stream_pause, and get rid of the underrun tracking code by updating bytes_played_last only for bytes we actually wrote.
(Assignee)

Comment 9

7 years ago
Created attachment 512098 [details] [diff] [review]
patch v0

Based on Yury's idea, but removing a bunch of extraneous existing code for resetting the time on resume and tracking underrun counts.

Testing sync.webm, A/V sync works during multiple headphone plug/unplug cycles, backgrounding the browser process, and sleeping the computer.

Since a dupe of this requested blocking, I think we should try to get this in to FF4.  Requesting approval.
Assignee: async.processingjs → kinetik
Attachment #512040 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #512098 - Flags: review?(chris.double)
Attachment #512098 - Flags: approval2.0?

Updated

7 years ago
Attachment #512098 - Flags: review?(chris.double) → review+
(Assignee)

Updated

7 years ago
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/5fa15207f617
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Matthew, can you confirm if this is fixed in the latest nightly Firefox 4.0b12pre?
(Assignee)

Comment 12

7 years ago
Done.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.