Closed Bug 870400 Opened 7 years ago Closed 7 years ago

audio distortion of tumblr videos in Firefox 21.0 (and FF22&23)

Categories

(Core :: Audio/Video, defect)

21 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla24
Tracking Status
firefox20 --- unaffected
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- verified
firefox24 --- verified

People

(Reporter: dusty-2011, Assigned: cpearce)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

In Firefox 20, the video playback on tumblr was choppy. And replaying the same video was broken. See bug 868630: https://bugzilla.mozilla.org/show_bug.cgi?id=868630.

In the latest nightly (23.0a1), the video playback on tumblr is no longer choppy. And replaying the same video is no longer broken. However, the audio of the tumblr videos now distorts. This audio problem was not present in Firefox 20, and is also not present in IE9 or Chrome 26.

Here are two sample videos, which clearly have distorted sound:
http://videohall.tumblr.com/post/45234276692/dairy-cows-in-holland-are-let-out-to-pasture-for#notes
http://videohall.tumblr.com/post/44032559382/car-goes-through-ice-ok#notes

The distortion is clearly noticeable in the first video, where the song Beautiful Day by U2 plays. There is a clear cracking sound noticeable throughout the video, a distortion. Also in the second tumblr video, the sound distorts in the latest nightly build. The person's voice is surrounded by cracking noises.
I have tested whether this sound distortion problem is present on other sites as well in the latest nightly build. I have tested these videos: 
https://www.youtube.com/watch?v=qqIIW7nxBgc
http://www.zie.nl/video/algemeen/Fors-meer-geevacueerden-Wetteren-dan-gedacht/m1nzerif0vja
http://vimeo.com/65576562

There were no audio distortion problems in the above videos. The audio distortion problems seems to be isolated to the tumblr video player.
Confirmed.
The video is choppy in FF 20.0.1.
The video is no longer choppy in FF 21 - 23, but the audio is distorted.
Status: UNCONFIRMED → NEW
Component: Untriaged → Video/Audio
Ever confirmed: true
Product: Firefox → Core
Firefox 21.0 was released today. Firefox 21.0 suffers from the distorted sound on the tumblr video player. Again, these are two samples: 
http://videohall.tumblr.com/post/45234276692/dairy-cows-in-holland-are-let-out-to-pasture-for#notes
http://videohall.tumblr.com/post/44032559382/car-goes-through-ice-ok#notes

It's a shame this bug is now present in the stable release version of Firefox.

Paul Silaghi, what are the next steps? We just wait forever until someone assigns himself to work on this bug?
Summary: audio distortion of tumblr videos on Firefox Nightly 23.0a1 → audio distortion of tumblr videos in Firefox 21.0 (and FF22&23)
Version: 23 Branch → 21 Branch
Based on comment 2 this is a regression. Can we get a regression range?
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/8f4ec9046b03
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130205 Firefox/21.0 ID:20130205021221
Bad:
http://hg.mozilla.org/mozilla-central/rev/1eab3e0d7f53
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130205 Firefox/21.0 ID:20130205072423
Pushlog;
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8f4ec9046b03&tochange=1eab3e0d7f53


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b894118e1c77
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130204 Firefox/21.0 ID:20130204132322
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/d4ce9051566e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130204 Firefox/21.0 ID:20130204134321
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b894118e1c77&tochange=d4ce9051566e

Triggered by:
d4ce9051566e	Chris Pearce — Bug 837859 - Enable Windows Media Foundation support for H.264,AAC and MP3 playback on Windows 7 and later. r=roc


The page uses the following video source:
[Prior to landing Bug 837859]
<object type="application/x-shockwave-flash" data="/javascript/video-js/video-js.swf" id="tumblr_video_player_44032559382_flash_api" name="tumblr_video_player_44032559382_flash_api" class="vjs-tech" width="100%" height="100%"><param name="movie" value="/javascript/video-js/video-js.swf"><param name="flashvars" value="readyFunction=_V_.flash.onReady&amp;eventProxyFunction=_V_.flash.onEvent&amp;errorEventProxyFunction=_V_.flash.onError&amp;autoplay=false&amp;preload=none&amp;loop=false&amp;muted=false&amp;src=http%3A%2F%2Fvideohall.tumblr.com%2Fvideo_file%2F44032559382%2Ftumblr_mit3i0lSV71r6ego3&amp;"><param name="allowScriptAccess" value="always"><param name="allowNetworking" value="all"><param name="wmode" value="opaque"><param name="bgcolor" value="#000000"></object>

[After landing Bug 837859]
<video tabindex="0" src="http://videohall.tumblr.com/video_file/44032559382/tumblr_mit3i0lSV71r6ego3" preload="none" id="tumblr_video_player_44032559382_html5_api" class="vjs-tech">
</video>

So I cannot say this is Firefox's fault.
Probably, I think that the html5 video source has a problem.
Thank you Alice!

I downloaded the video at http://videohall.tumblr.com/video_file/44032559382/tumblr_mit3i0lSV71r6ego3 and it plays fine in Windows Media Player, so we must be doing something wrong here in our H.264/AAC backend.
(In reply to Alice0775 White from comment #5)
> So I cannot say this is Firefox's fault.
> Probably, I think that the html5 video source has a problem.
This is wrong.

Steps To Reproduce:
1. Open http://videohall.tumblr.com/video_file/44032559382/tumblr_mit3i0lSV71r6ego3

Actual Results:
 *Sound has a problem in Firefox21-24
 *Works ok in Opera12.15 and IE9 (QuickTime plugin both)

So, this is definitely Firefox 'Windows Media Foundation' problem
The problem with these videos is that their audio tracks are timestamped wrong.

Each audio IMFSample reports its duration as approximately 2 more samples than it actually has, and this results in us writing silence to the AudioStream to make up the missing samples, which is the crackling we hear. This sample underrun seems to  also be reflected in the timestamp, which is cumulatively off by about 2 more samples for every IMFSample. The videos are mono, 8bit PCM, and encoded by Lavf54.59.106, which presumably must be buggy.

I guess we should calculate the audio samples' timestamps ourselves based on the sum of the samples, and reset the count when we encounter a sample with its MFSampleExtension_Discontinuity attribute set.
Calculate audio timestamps/duration of WMF audio samples based on the number of samples decoded, rather than trusting WMF's timestamps.
Assignee: nobody → cpearce
Attachment #751671 - Flags: review?(paul)
Attachment #751671 - Flags: review?(paul) → review+
Would a test be easy enough to write ?
(In reply to Paul Adenot (:padenot) from comment #10)
> Would a test be easy enough to write ?

hmmm... We'd need to create a buggy file and then capture the decoded output and check that we're not injecting silence into it to cover the gaps...
Maybe using Web Audio and |decodeAudioData| (since that uses WMF)+ AnalyzerNode. Maybe it's not worth the trouble.
And assertions.
https://tbpl.mozilla.org/php/getParsedLog.php?id=23255623&full=1&branch=mozilla-inbound#error0

04:22:35     INFO -  [Parent 3568] ###!!! ASSERTION: Seek target should lie inside the first audio block after seek: '!audio || (audio->mTime <= seekTime && seekTime <= audio->mTime + audio->mDuration)', file e:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/content/media/MediaDecoderStateMachine.cpp, line 1959
So the problem here is that when we seek(0), WMF doesn't mark the stream as a discontinuity, so we don't re-capture the audio frame counts after a seek(0), and thus the timestamps are double what they're supposed to be on the second play through, hence the failure in test_play_twice. We need to re-capture the audio frame counts after a seek.
Reset the audio sample/sum after a seek. This is based on top of the previous patch. I'll merge this into the previous patch before landing, but I'm putting it up separately for easier review.
Attachment #754657 - Flags: review?(paul)
Attachment #754657 - Flags: review?(paul) → review+
https://hg.mozilla.org/mozilla-central/rev/77a569c522b1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
This is fixed in the current Nightly, thanks!! The sample videos in my original post playback smoothly and with good sound.

Any chance this can get uplifted to Firefox 22 and 23? I would hate having to wait for Firefox 24 to release; and having to use a different browser for tumblr in the meantime.
(In reply to dusty-2011 from comment #20)
> Any chance this can get uplifted to Firefox 22 and 23?
It depends on the benefit vs. risk ratio.
(In reply to Scoobidiver from comment #21)
> (In reply to dusty-2011 from comment #20)
> > Any chance this can get uplifted to Firefox 22 and 23?
> It depends on the benefit vs. risk ratio.

In that case, I would like to comment on the benefits: there are thousands of tumblr blogs which use the same type of videos (with wrong audio timestamps), all of these videos (probably millions of them) have distorted sound in Firefox 21.

In fact, I think it is the standard video player of tumblr which exihibits this behavior (timestamping the videos wrong), so all videos that bloggers upload for use on this standard video player play with distorted sound in Firefox 21.

So there are clear benefits for uplifting this to Firefox 23 or 22 for the millions of tumblr users and bloggers.

I'm not an expert on the risks of uplifting this patch to aurora or beta, you would have to ask someone else about this. I just wanted to point out that there are very significant benefits of uplifting to aurora or beta.
Verified fixed nightly 24.0a1 (2013-06-02) win 7 x64
Status: RESOLVED → VERIFIED
Rolled up patch, that I landed on m-i.
Attachment #751671 - Attachment is obsolete: true
Attachment #754657 - Attachment is obsolete: true
Attachment #757674 - Flags: review+
Comment on attachment 757674 [details] [diff] [review]
Patch: Calculate audio timestamps based on samples written

[Approval Request Comment]
Bug caused by (feature/regressing bug #): H.264/AAC video support on Windows
User impact if declined: "Lots" of videos on tumblr won't play correctly, they'll have crackles in their audio output.
Testing completed (on m-c, etc.): This has been on Nightly for a few days.
Risk to taking this patch (and alternatives if risky): Low.
String or IDL/UUID changes made by this patch: None.
Attachment #757674 - Flags: approval-mozilla-aurora?
Attachment #757674 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed FF 23.0a2 (2013-06-09) Win 7 x64
You need to log in before you can comment on or make changes to this bug.