Closed Bug 1047180 Opened 10 years ago Closed 10 years ago

Add HE-AAC support to MP4Reader

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: ajones, Assigned: jya)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 4 obsolete files)

Steps to reproduce:

* Enable MP4 reader prefs
* Navigate to link
* Press play

Expected results:

Normal playback

Actual results:

Half speed playback

The issue is that the MP4 container reports 24K sample rate but we need to get the sample rate from the decoder which knows the correct sample rate.
Assignee: nobody → jyavenard
Another testcase: http://www-tvline-com.vimg.net/streaming/tvline/MAA101-Medianet.mp4

Ralph tried this with his MacOSX backend, and it seemed to work there, so maybe the MacOSX backend is not using SBS/PS and so returning the lower quality output?
The basic fix here should be:

* Add a rate and channels field to AudioData. There's a bit of work here to change the existing call sites of the AudioData constructor in all backends to pass in this new data.
* In MP4Reader::ReadMetadata(), after we've setup the MediaDataDecoders, we should decode one audio sample and check the audio rate and number of channels. Report that in the MediaInfo of MP4Reader::ReadMetadata().

We prefer adding the rate and channels fields to AudioData over adding a callback to report stream changes because we already have this precedent set by video stream changes, and because we want to move towards having lightweight AudioStreams, where all audio sources playing pass in audio samples of arbitrary rates, and the AudioStream mixes them to the output rate. If we use callbacks, we would need to forward the callback all the way up the stack, and then keep track of which audio samples were before/after the rate change, so we'd need basically these fields anyway.
Note on why it works on OSX...

OSX doesn't use the sampling rate provided by the demuxer.
Instead it uses its own probe method AudioFileStreamParseBytes ; which actually also detects the audio as being 24kHz.
FFmpeg decoder returns the raw PCM data, while OSX decoder outputs data in the format/rate requested here 22kHz and perform on the fly resampling if required.
So on mac it sounds good.

I haven't figured out a way to determine the actual sampling rate using CoreAudio AudioConverter...
it would probably allow for better audio quality as we wouldn't lose half the audio resolution
Comment on attachment 8469971 [details] [diff] [review]
Decode a single audio frame in order to retrieve accurate channel count and sampling rate and propagate to MP4Reader

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

::: content/media/fmp4/MP4Reader.cpp
@@ +348,5 @@
>      NS_ENSURE_SUCCESS(rv, rv);
> +
> +    // Decode one audio sample to detect potentially incorrect channels count or
> +    // sampling rate from demuxer.
> +    nsAutoPtr<MP4Sample> audioSample(PopSample(kAudio));

You should probably just call Decode(kAudio) in a loop while AudioQueue() is empty and an error or EOS isn't hit.

::: content/media/fmp4/wmf/WMFAudioMFTManager.cpp
@@ +234,5 @@
>                             timestamp,
>                             duration,
>                             numFrames,
>                             audioData.forget(),
> +                           mAudioChannels, mAudioRate);

The patch in bug 1050064 should handle the WMFAudioMFTManager.
Attachment #8469971 - Attachment is obsolete: true
Attachment #8470577 - Attachment is obsolete: true
Attachment #8470577 - Flags: review?(cpearce)
Comment on attachment 8470578 [details] [diff] [review]
Decode a single audio frame in order to retrieve accurate channel count and sampling rate and propagate to MP4Reader

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

Looks good.

::: content/media/fmp4/MP4Reader.cpp
@@ +596,5 @@
> +        LOG("MP4Reader::Output change of sampling rate:%d->%d",
> +            mInfo.mAudio.mRate, audioData->mRate);
> +        mInfo.mAudio.mRate = audioData->mRate;
> +        mInfo.mAudio.mChannels = audioData->mChannels;
> +        // TODO What do we do when we have a format change?

We don't do anything now, we will let the MediaDecoderStateMachine handle it the change when it come to play this data.

So you can remove this comment.

If we encounter a stream change in anything other than the first sample, we should call mDecoder->QueueMetadata() (so that JS gets a "loadedmetadata" event dispatched to the media element), but until we find an example of a file that causes that, we should not do that yet.
Attachment #8470578 - Flags: review?(cpearce) → review+
Attachment #8470578 - Attachment is obsolete: true
Blocks: 1022501
Blocks: 1050064
No longer depends on: 1050064
Blocks: 1052383
Attachment #8470633 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c869d969bc12
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Blocks: 1055841
You need to log in before you can comment on or make changes to this bug.