1166760 - [Aries] Videos on Facebook play back with extremely slow audio

Status: VERIFIED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[Chris Lord [:cwiiis]]

STR:

Prerequisite: Aries device (tested on a Sony Xperia Z3C) with Facebook installed, logged in and a news feed with video links in it.

1. Locate a video on the Facebook news feed and tap it to watch it

Expected:

Video opens in new window and plays back normally.

Actual:

Video opens in new window and plays, but audio is very slow and pitched down (like slowed down 2-4x). When the video ends, the audio will continue playing and the time indicator starts to increment beyond the end of the video.

This does not happen on a Flame or ZTE Open C last time I tried. YouTube videos play back fine, this seems specific to whatever format Facebook uses.

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

[Blocking Requested - why for this release]:

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Can you take a look at this please?  It seems like it might be gonk related to the aries build.

Comment 3

[Kai-Zhen Li [:seinlin][:kli]]

I can't reproduce this on aries-l. But it is reproduced on aries-kk.

Comment 4

[Kai-Zhen Li [:seinlin][:kli]]

Bwu, With aries-kk, Video on youtube can be played correctly but playing video on facebook will get this audio issue. Do you have any suggestion?

Comment 5

[Kai-Zhen Li [:seinlin][:kli]]

Alexandre, I am not sure if any library is missing in aries-kk. Do you get an idea?

Comment 6

[:gerard-majax]

No. And this bug is totally useless without any build informations.

Comment 7

[Blake Wu [:bwu][:blakewu]]

(In reply to Kai-Zhen Li [:kli][:seinlin] from comment #4)
> Bwu, With aries-kk, Video on youtube can be played correctly but playing
> video on facebook will get this audio issue. Do you have any suggestion?

Munro,
May I have your help to check it?
Thanks!

Comment 8

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

This should be related to bug 1167500.
GonkAudioDecoderManager got the wrong sample rate.

Comment 9

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: lW2ft.mp4

Comment 10

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

I can reproduce the bug with playing attachment 8612786 [details].
The clips audio track contains 4 channel.

Format                                   : AAC
Format profile                           : HE-AACv2 / HE-AAC / LC
Channel(s)                               : 2 channels / 1 channel / 1 channel
Channel positions                        : Front: L R / Front: C / Front: C
Sampling rate                            : 48.0 KHz / 48.0 KHz / 24.0 KHz

MediaFormatReader configure channel# to 1 and sample rate to 24000.
But actually we are playing 2 48 Khz channels.

Blake,

Could you help check the demuxer behavior?

Comment 11

[:gerard-majax]

That's fun, I already ran into issues on some devices (including Xperia ZR) where decoding was being sent from Gecko to libstagefright with bogus mismatching sample rate, like 44 KHz when the stream was 48 KHz.

Comment 12

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Recently there are several patches landed which refactor media reader design in KK m-c. Need to cowork with Jean & Blake to figure out the problem.

Comment 13

[Jean-Yves Avenard [:jya]]

(In reply to Munro Chiang [:mchiang] from comment #10)
> I can reproduce the bug with playing attachment 8612786 [details].
> The clips audio track contains 4 channel.
> 
> Format                                   : AAC
> Format profile                           : HE-AACv2 / HE-AAC / LC
> Channel(s)                               : 2 channels / 1 channel / 1 channel
> Channel positions                        : Front: L R / Front: C / Front: C
> Sampling rate                            : 48.0 KHz / 48.0 KHz / 24.0 KHz
> 
> MediaFormatReader configure channel# to 1 and sample rate to 24000.
> But actually we are playing 2 48 Khz channels.
> 
> Blake,
> 
> Could you help check the demuxer behavior?

The problem is not in the demuxer nor the MediaFormatReader (you would have had the exact same behaviour with the older MP4Reader).

The MediaFormatReader will read the metadata from the container, what is typically found there is the core AAC-LC stream information, so that's where your 24kHz sampling rate is coming from.
Many AAC streams have the SBR inband, as such the demuxer isn't aware that it's HE-AAC.

As such, we have to decode the first audio frame first. The MediaDataDecoder will typically detect the SBR data and return a frame with the updated sampling rate: 48kHz.

It is expected that the MediaDataDecoder will wait until either the outband SBR (stored in the container and passed in mCodecSpecificData) or the inband SBR has been found before outputting a decoded audio sample.

If the gonk decoder doesn't detect that it's an HE-AAC stream in the first frame but only does so in the following frames, there will be a mismatch of configuration, and the audio will not play properly.

Glancing at the code, it appears that the gonk audio decoder set the audio rate to what is set in the demuxer (and stored in mAudioRate) and at no time queries the codec to see if the actual frame audio frame returned is of a different sampling rate. It assumes the sampling rate is the same.

This is the reason why this file doesn't play properly.

The gonk decoder needs to query the updated format.
The OMX MediaCodecReader appears to do it properly http://mxr.mozilla.org/mozilla-central/source/dom/media/omx/MediaCodecReader.cpp#1470

And more particularly there:
http://mxr.mozilla.org/mozilla-central/source/dom/media/omx/MediaCodecReader.cpp#1507

The Gonk MediaDataDecoder needs to perform a similar operation

Comment 15

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

FYI Pandora is affected ( https://bugzilla.mozilla.org/show_bug.cgi?id=1167500#c4 )as it uses AAC encoding.

Comment 16

[Blake Wu [:bwu][:blakewu]]

(In reply to Munro Chiang [:mchiang] from comment #10)
> I can reproduce the bug with playing attachment 8612786 [details].
> The clips audio track contains 4 channel.
> 
> Format                                   : AAC
> Format profile                           : HE-AACv2 / HE-AAC / LC
> Channel(s)                               : 2 channels / 1 channel / 1 channel
> Channel positions                        : Front: L R / Front: C / Front: C
> Sampling rate                            : 48.0 KHz / 48.0 KHz / 24.0 KHz
> 
> MediaFormatReader configure channel# to 1 and sample rate to 24000.
> But actually we are playing 2 48 Khz channels.
Thanks for your help to narrow down the possible root causes!

Comment 17

[Blake Wu [:bwu][:blakewu]]

(In reply to Jean-Yves Avenard [:jya] from comment #13)
> (In reply to Munro Chiang [:mchiang] from comment #10)
> > I can reproduce the bug with playing attachment 8612786 [details].
> > The clips audio track contains 4 channel.
> > 
> > Format                                   : AAC
> > Format profile                           : HE-AACv2 / HE-AAC / LC
> > Channel(s)                               : 2 channels / 1 channel / 1 channel
> > Channel positions                        : Front: L R / Front: C / Front: C
> > Sampling rate                            : 48.0 KHz / 48.0 KHz / 24.0 KHz
> > 
> > MediaFormatReader configure channel# to 1 and sample rate to 24000.
> > But actually we are playing 2 48 Khz channels.
> > 
> > Blake,
> > 
> > Could you help check the demuxer behavior?
> 
> The problem is not in the demuxer nor the MediaFormatReader (you would have
> had the exact same behaviour with the older MP4Reader).
> 
> The MediaFormatReader will read the metadata from the container, what is
> typically found there is the core AAC-LC stream information, so that's where
> your 24kHz sampling rate is coming from.
> Many AAC streams have the SBR inband, as such the demuxer isn't aware that
> it's HE-AAC.
> 
> As such, we have to decode the first audio frame first. The MediaDataDecoder
> will typically detect the SBR data and return a frame with the updated
> sampling rate: 48kHz.
> 
> It is expected that the MediaDataDecoder will wait until either the outband
> SBR (stored in the container and passed in mCodecSpecificData) or the inband
> SBR has been found before outputting a decoded audio sample.
> 
> If the gonk decoder doesn't detect that it's an HE-AAC stream in the first
> frame but only does so in the following frames, there will be a mismatch of
> configuration, and the audio will not play properly.
> 
> Glancing at the code, it appears that the gonk audio decoder set the audio
> rate to what is set in the demuxer (and stored in mAudioRate) and at no time
> queries the codec to see if the actual frame audio frame returned is of a
> different sampling rate. It assumes the sampling rate is the same.
> 
> This is the reason why this file doesn't play properly.
> 
> The gonk decoder needs to query the updated format.
> The OMX MediaCodecReader appears to do it properly
> http://mxr.mozilla.org/mozilla-central/source/dom/media/omx/MediaCodecReader.
> cpp#1470
> 
> And more particularly there:
> http://mxr.mozilla.org/mozilla-central/source/dom/media/omx/MediaCodecReader.
> cpp#1507
> 
> The Gonk MediaDataDecoder needs to perform a similar operation
Thanks for your detailed information!  
I will check it.

Comment 18

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

I have come out some idea to mimic the way MediaOmxReader query the sample rate & channel# from codec. Will provide a WIP in these two days.

However, it seems current design of Android MediaCodec doesn't expose a method for upstream to query these info. That means in Android, extractor takes the whole responsibility to parse the correct sample rate & channel# info.

Comment 19

[Jean-Yves Avenard [:jya]]

(In reply to Munro Chiang [:mchiang] from comment #18)
> That means in Android, extractor
> takes the whole responsibility to parse the correct sample rate & channel#
> info.

This can't work. The container doesn't contain any information about the AAC-HE stream

Comment 20

[Doug Sherk (:drs) (inactive)]

Sotaro, do you have any suggested next steps here, or people we can contact?

Comment 22

[Daniel Holbert [:dholbert]]

(FWIW my dupe bug 1171179 reproduces on the Flame as well as the Aries device, which semi-conflicts with comment 0 here.  Not sure if that means comment 0 was mistaken, or if bug 1171179 is not a dupe after all, or if only certain videos trigger this on the flame & I happen to have found one.)

Comment 23

[Sotaro Ikeda [:sotaro]]

(In reply to Doug Sherk (:drs) (use needinfo?) from comment #20)
> Sotaro, do you have any suggested next steps here, or people we can contact?

The following functions already do necessary things for android::MediaCodec. Jut mimic them seems enough to fix the problem. android::MediaCodec::getOutputFormat() provides necessary metadata.
 - MediaCodecReader::EnsureCodecFormatParsed()
 - MediaCodecReader::UpdateAudioInfo()

https://dxr.mozilla.org/mozilla-central/source/dom/media/omx/MediaCodecReader.cpp#1554

Comment 24

[Sotaro Ikeda [:sotaro]]

MediaCodecReader::UpdateVideoInfo() equivalent is also necessary.

Comment 25

[Doug Sherk (:drs) (inactive)]

Thanks, Sotaro. Munro, hopefully this information is helpful. I'm assigning this to you since you're actively working on this. Please let me know if this is a problem, or you've stopped working on it.

Comment 26

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: WIP_query_output_format.patch

Comment 27

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

(In reply to Sotaro Ikeda [:sotaro] from comment #23)
> (In reply to Doug Sherk (:drs) (use needinfo?) from comment #20)
> > Sotaro, do you have any suggested next steps here, or people we can contact?
> 
> The following functions already do necessary things for android::MediaCodec.
> Jut mimic them seems enough to fix the problem.
> android::MediaCodec::getOutputFormat() provides necessary metadata.
>  - MediaCodecReader::EnsureCodecFormatParsed()
>  - MediaCodecReader::UpdateAudioInfo()
> 
> https://dxr.mozilla.org/mozilla-central/source/dom/media/omx/
> MediaCodecReader.cpp#1554

Thanks for the information!
I port the snippet to GonkAudioDecoderManager.cpp.
Please let me know if there is any concern.

Comment 28

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: bug-1166760-fix.patch

Comment 29

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8615767 [details] [diff] [review]
bug-1166760-fix.patch

Looks good!

Comment 30

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=858896f9b551

Comment 31

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/56cb10f6dc13

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/56cb10f6dc13

Comment 33

[Martin Shuman [:Marty]]

This issue is verified fixed on the latest Spark and Flame 2.5 builds.
Pandora and Facebook audio plays at the proper speed and sounds appropriate.

Environmental Variables:
Device: Flame 2.5
BuildID: 20150818030206
Gaia: d9d99f32762975a370f1abd34a3512bd6fe29111
Gecko: 90d9b7c391d38ae118865bd87b5d011feee6dded
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Environmental Variables:
Device: Aries 2.5
BuildID: 20150818175310
Gaia: 507ba38fb64b27f87d11f4104dfcc58448e12b1a
Gecko: 2c272af993c23e803f6ea7798a812b0c8abfad4d
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0