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

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Caspy7]

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

Comment 1

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

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

Comment 2

[Caspy7]

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

Comment 3

[Caspy7]

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?

Comment 4

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

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

Comment 5

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

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

Comment 6

[Caspy7]

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

Comment 7

[Caspy7]

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

Comment 8

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

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.

Comment 9

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

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

Comment 10

[Caspy7]

So then is part of the problem with videos reporting an incorrect audio sample rate?

Comment 11

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

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

Comment 12

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

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

Comment 13

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

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.

Comment 15

[jazza cook]

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.

Comment 16

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

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 17

[Paul Adenot (:padenot)]

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.

Comment 18

[[e-mail]]

any progress?

Comment 19

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

This requires infrastructure work for it to be reliable. I'm doing that work now, but it will take time.

Comment 20

[lobacten]

Is this still in progress, or has it been abandoned?

Comment 21

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

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.

Comment 22

[Caspy7]

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.