WMF backend gets incorrect sample rate for HE-AAC audio stream with SBR/PS

RESOLVED FIXED

Status

()

RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: caspy77, Assigned: cpearce)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Play the video on this page:
http://tvline.com/2013/05/21/marvels-avengers-assemble-disney-xd-premiere-dates-video/
In Firefox the audio plays slowly (obvious once you see characters speaking dialog) but in Chrome, Opera and IE it works fine.

Got the same results in the most recent Aurora, Beta and Nightly (win8).
(Assignee)

Comment 1

6 years ago
Thanks, this is a bug in our HTML5 H.264 video backend.
Reduced testcase:
http://www-tvline-com.vimg.net/streaming/tvline/MAA101-Medianet.mp4

Updated

6 years ago
Blocks: 837859
(Reporter)

Updated

6 years ago
Summary: HTML5 audio is slowed down on this video. Works in other browser → HTML5 audio is slowed down on this video. Works in other browsers
(Reporter)

Comment 2

6 years ago
Created attachment 753926 [details]
VLC Media info on the left. Windows 7 file properties on the right.

The VLC media properties for this video indicate 48 kHz while the windows file properties indicate 24 kHz.
I'm not highly knowledgeable on this topic, but wondering if this has anything to do with the issue.
(Reporter)

Comment 3

6 years ago
Sorry, in comment 2 I somehow missed stating that those conflicting kHz rates refer to the audio sample rate.

Also, sometimes (it seems often on the first load) there won't be any audio at all.

Additionally, there is a video distortion I'm seeing at the bottom of the video (stretching the last pixels straight down). Is anybody else seeing this? 
Should a separate bug be filed for that or is it likely enough to be fixed with this one to leave it alone?
(Assignee)

Comment 4

6 years ago
The problem here is that the audio stream is HE-AAC, and according to the AAC decoder's documentation [1] for HE-AAC WMF reports "the sample rate and number of channels prior to the application of spectral band replication (SBR) and parametric stereo (PS) tools, if present. The effect of the SBR tool is to double the decoded sample rate relative to the core AAC-LC sample rate. The effect of the PS tool is to decode stereo from a mono-channel core AAC-LC stream."

We're using the samplerate and number of channels to open the cubeb audio stream, and presumably that means we're opening it at 1/4 or 1/2 of the actual sample rate that WMF is outputting.

I'm not sure how to reliably detect HE-AAC with SBR/PS yet; that audio stream's IMFMediaType has its MF_MT_AAC_AUDIO_PROFILE_LEVEL_INDICATION attribute set to 0, but I'd expect it to be in the range [0x2c,0x33] (or maybe [0x30,0x33], not sure which) according to [2].

[1] http://msdn.microsoft.com/en-us/library/windows/desktop/dd742784%28v=vs.85%29.aspx
[2] http://www.itscj.ipsj.or.jp/sc29/open/29view/29n6475t.doc
Summary: HTML5 audio is slowed down on this video. Works in other browsers → WMF backend gets incorrect sample rate for HE-AAC audio stream with SBR/PS
(Assignee)

Comment 5

6 years ago
(In reply to Mark S. from comment #3)
> Additionally, there is a video distortion I'm seeing at the bottom of the
> video (stretching the last pixels straight down). Is anybody else seeing
> this? 

Are you testing in Nightly or Aurora? This bug exists on Aurora as bug 872375, but is fixed in Nightly builds. Do you still see this bug in an up-to-date Nightly build? Thanks!
(Reporter)

Comment 6

6 years ago
(In reply to Chris Pearce (:cpearce) from comment #4)
> I'm not sure how to reliably detect HE-AAC with SBR/PS yet

This is completely out of my league, but would there be any benefit to looking at chromium's code for this?

(In reply to Chris Pearce (:cpearce) from comment #5)
> (In reply to Mark S. from comment #3)
> > Additionally, there is a video distortion I'm seeing at the bottom of the
> > video (stretching the last pixels straight down). Is anybody else seeing
> > this? 
> 
> Are you testing in Nightly or Aurora? This bug exists on Aurora as bug
> 872375, but is fixed in Nightly builds. Do you still see this bug in an
> up-to-date Nightly build? Thanks!

I'm using Aurora 23. I'll have to try it out in Nightly.
(Reporter)

Comment 7

6 years ago
(In reply to Chris Pearce (:cpearce) from comment #5)
> Are you testing in Nightly or Aurora? This bug exists on Aurora as bug
> 872375, but is fixed in Nightly builds. Do you still see this bug in an
> up-to-date Nightly build? Thanks!

Indeed, that bug looks to be fixed on Nightly.
(Assignee)

Comment 8

6 years ago
Got it. The IMFReader is reporting the sample rate at half what it comes out at for this file. When WMF starts to decode that file, it actually is returning a flag MF_SOURCE_READERF_CURRENTMEDIATYPECHANGED on the first call to IMFSourceReader::ReadSample() for an audio sample read, and after that the reported sample rate is the rate that the reader's outputting, i.e. it's what we need.

The reason why I wasn't picking this up is becausethe first call to DecodeAudioData() happens inside MediaDecoderReader::FindStartTime(), which assumes MF_SOURCE_READERF_CURRENTMEDIATYPECHANGED is a failure, but that failure isn't propagated to DecodeLoop(), so we still play. And the next time we call WMFReader::DecodeAudioData() the media type isn't changing, so we don't get another notification.

The fix is simple; we should decode the first audio sample in ReadMetadata() and check if the media type changes. If the media type changes during normal playback we can still treat it as an error, since then we'd have to tear down and restart the audio stream at a new bit rate, and it's not something that we need to support for the use cases we're covering.
(Assignee)

Comment 9

6 years ago
Created attachment 757814 [details] [diff] [review]
Patch

Decode one audio sample, and then retrieve the sample rate and number channels etc. This ensures that SBR/PS has time to be signaled to the decoder so that the current media type's sample rate is what the decoder outputs.
Assignee: nobody → cpearce
Attachment #757814 - Flags: review?(paul)
(Reporter)

Comment 10

6 years ago
So then is part of the problem with videos reporting an incorrect audio sample rate?
(Assignee)

Comment 11

6 years ago
Windows' AAC decoder reports the sample rate without taking into account some of the factors that are implicitly signaled in the encoded samples, not the metadata. So we need to decode a bit to get the actual decoded sample rate.

http://msdn.microsoft.com/en-us/library/windows/desktop/dd742784%28v=vs.85%29.aspx
(Assignee)

Comment 12

6 years ago
Comment on attachment 757814 [details] [diff] [review]
Patch

Sorry, clearing review request, this is making test_bug493187.html nearly perma-orange timeout:

https://tbpl.mozilla.org/?tree=Try&rev=4d2f0f1b0ccd
Attachment #757814 - Flags: review?(paul)
(Assignee)

Comment 13

6 years ago
Comment on attachment 757814 [details] [diff] [review]
Patch

I'll fixing the underlying issue that's causing the orange when landing patch in bug 882537, so re-requesting review on this patch. I'll land this patch and bug 882537 together, once I've got review on the patches I'll upload there.
Attachment #757814 - Flags: review?(paul)
(Assignee)

Updated

6 years ago
Depends on: 882537
(Assignee)

Updated

6 years ago
Duplicate of this bug: 888763

Comment 15

6 years ago
Just to say https://bugzilla.mozilla.org/show_bug.cgi?id=888763 concerns m4a audio files only. I've had no problems with mp4 videos so far. Thanks.
(Assignee)

Comment 16

6 years ago
MP4 and M4A use the same type of audio codec, and this problem only occurs in content encoded with one particular variant/encoding mode using that audio codec.
Comment on attachment 757814 [details] [diff] [review]
Patch

Review of attachment 757814 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/wmf/WMFReader.cpp
@@ +505,5 @@
>  
> +  // Read one audio sample. HE-AAC streams may report their samples rates and
> +  // number of channels without taking the effects of Spectral Band
> +  // Replication and Parametric Stereo into account. Once a sample is decoded,
> +  // these are properly (perhaps implicitly) signalled to the decoder, and the

s/signalled/signaled/

@@ +542,5 @@
>  
> +  // Enqueue the sample in the audio queue. We must do this after the
> +  // audio stream properties above are initialized.
> +  if (!EnqueueAudioSample(timestampHns, sample)) {
> +    // Audio stream must have a single sample.    

trailing space.

@@ +703,5 @@
>    }
>  
> +  if (!EnqueueAudioSample(timestampHns, sample)) {
> +    // Error, or end of stream.
> +    mAudioQueue.Finish(); 

Trailing space.
Attachment #757814 - Flags: review?(paul) → review+

Comment 18

5 years ago
any progress?

Updated

5 years ago
Flags: needinfo?(cpearce)
(Assignee)

Comment 19

5 years ago
This requires infrastructure work for it to be reliable. I'm doing that work now, but it will take time.
Flags: needinfo?(cpearce)

Comment 20

4 years ago
Is this still in progress, or has it been abandoned?
Flags: needinfo?(cpearce)
(Assignee)

Comment 21

4 years ago
It's fixed for me in Firefox 35 on Windows thanks to the MP4Reader and associated changes. Please retest and re-open if that's not the case for you.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Flags: needinfo?(cpearce)
(Reporter)

Comment 22

4 years ago
I just tried the video testcase in comment 1 and it worked just fine (36b1).  So unless they updated the video, then it seems fixed.
If anyone has a video with the same problem matching the description here, please post. Otherwise it appears resolved.
You need to log in before you can comment on or make changes to this bug.