520101 - Audio/video sync lost when hotswapping sound output

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Matthew Gregan [:kinetik]]

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

Comment 1

[Matthew Gregan [:kinetik]]

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.

Comment 3

[Matthew Gregan [:kinetik]]

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.

Comment 4

[Matthew Gregan [:kinetik]]

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.

Comment 5

[David Humphrey (:humph)]

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;

Comment 6

[David Humphrey (:humph)]

Attachment: 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.

Comment 8

[Matthew Gregan [:kinetik]]

(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.

Comment 9

[Matthew Gregan [:kinetik]]

Attachment: 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.

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

http://hg.mozilla.org/mozilla-central/rev/5fa15207f617

Comment 11

[u279076]

Matthew, can you confirm if this is fixed in the latest nightly Firefox 4.0b12pre?

Comment 12

[Matthew Gregan [:kinetik]]

Done.