1032678 - [tarako]Enter the audio player, music app can't load AMR-NB, AMR-WB audio files

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[jingmei.zhang]

Attachment: armNB-and-armWB.zip

1.pull the AMR-NB or AMR-WB into the SD card.
2.open the music app and the app can't load the audio file.

I have put the relevant AMR-NB , AMR-WB audio files in the attachment.

Comment 1

[Sri Kasetti]

We do support AMR formats..
Dominic, Can you take a look at this issue?

Comment 2

[James Zhang (Spreadtrum)]

It caused by bug 1002897 and bug 1010434.

Comment 3

[James Zhang (Spreadtrum)]

(In reply to James Zhang (Spreadtrum) from comment #2)
> It caused by bug 1002897 and bug 1010434.

Jingmei, right?

Comment 4

[jingmei.zhang]

(In reply to James Zhang (Spreadtrum) from comment #3)
> (In reply to James Zhang (Spreadtrum) from comment #2)
> > It caused by bug 1002897 and bug 1010434.
> 
> Jingmei, right?

In my side,it seems to be so.
I remove the code in window_manager.js:
var musicManifestUrl =
  protocol + 'music.' + domain + '/manifest.webapp';
var outOfProcessBlackList = [
  ……
 musicManifestUrl
];
 then the music app could display the amr audio files well.

Comment 5

[Dominic Kuo [:dkuo]]

(In reply to Sri Kasetti from comment #1)
> We do support AMR formats..
> Dominic, Can you take a look at this issue?

Working on it, update later.

Comment 6

[Dominic Kuo [:dkuo]]

Here are the logs that I have when inproc music while parsing the amr files:
--------------------------------------------------------------------------------
[JavaScript Warning: "Media resource blob:4a02ffc3-8a1c-4449-b27c-ac050768b979 could not be decoded." {file: "app://music.gaiamobile.org/index.html#mix" line: 0}]

Content JS WARN at app://music.gaiamobile.org/shared/js/mediadb.js:1772 in metadataError: MediaDB: error parsing metadata for /extsdcard/Music/amr/100k_rich_AMR_17.amr : Unplayable music file
--------------------------------------------------------------------------------
Looks like the audio element couldn't decode them so the mediadb just marked all the amr files as "Unplayable music file".

And if I remove the musicManifestUrl from the black list in window manager, like Jingmei said in comment 4, this issue does not occur, this is weird because it seems the url might affect the audio element to decode amr files.
 	
Bruce, since you have fixed bug 990957 that related to the amr files on tarako, would you please give us some hint base on the above logs? thanks.

Comment 7

[Wayne Chang [:wchang]]

broken feature, lets fix it.

Comment 8

[Hema Koka [:hema]]

Bruce/Eric,

Can you please help here (see comment 6)

Thanks
Hema

Comment 9

[Bruce Sun [:brsun]]

Update: MediaExtractor::countTracks() returns zero and OmxDecoder::Init() returns false in this case.
 - http://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/file/79dd08ba148f/content/media/omx/OmxDecoder.cpp#l369

Comment 10

[Bruce Sun [:brsun]]

Update: FileMediaResource::Tell() returns zero even when nsISeekableStream::Tell() returns NS_BASE_STREAM_CLOSED due to nsFileStreamBase::mFD is nullptr.

Comment 11

[Bruce Sun [:brsun]]

The root cause of this issue seems the same as bug 1010685. This issue can be fixed by applying the same change as attachment 8424704 [details] [diff] [review].

Comment 12

[Eric Chou [:ericchou] [:echou]]

(In reply to Bruce Sun [:brsun] from comment #11)
> The root cause of this issue seems the same as bug 1010685. This issue can
> be fixed by applying the same change as attachment 8424704 [details] [diff] [review]
> [review].

Thank you, Bruce.

Wayne,

according to comment 11, if we want to fix it for 1.3T, you may need to nominate bug 1010685 as a 1.3T+ or cherry-pick the patch to partner's codebase to fix this problem. Please assist with taking over it. 

Thank you.

Comment 13

[Bruce Sun [:brsun]]

Attachment: bug1032678_merge_bug1010685.patch

Merge the patch from bug 1010685 to 1.3t.

Comment 14

[Eric Chou [:ericchou] [:echou]]

(In reply to Eric Chou [:ericchou] [:echou] from comment #12)
> (In reply to Bruce Sun [:brsun] from comment #11)
> > The root cause of this issue seems the same as bug 1010685. This issue can
> > be fixed by applying the same change as attachment 8424704 [details] [diff] [review]
> > [review].
> 
> Thank you, Bruce.
> 
> Wayne,
> 
> according to comment 11, if we want to fix it for 1.3T, you may need to
> nominate bug 1010685 as a 1.3T+ or cherry-pick the patch to partner's
> codebase to fix this problem. Please assist with taking over it. 
> 
> Thank you.

Since Bruce has provided a 1.3T-specific patch, no need to uplift bug 1010685 then.

Comment 15

[Bruce Sun [:brsun]]

Attachment: bug1032678_merge_bug1010685.checkin-needed.patch

TBPL results: https://tbpl.mozilla.org/?tree=Try&rev=42a274002252

Comment 16

[Carsten Book [:Tomcat]]

Bruce: do you know if the test failure on debug from debug on this try run are related to this patch ?

Comment 17

[Bruce Sun [:brsun]]

(In reply to Carsten Book [:Tomcat] from comment #16)
> Bruce: do you know if the test failure on debug from debug on this try run
> are related to this patch ?

Carsten: none of any commits on 1.3t branch run tests on "B2G ICS Emulator Debug" build
 - https://tbpl.mozilla.org/?tree=Mozilla-B2g28-v1.3t

Comment 18

[Bruce Sun [:brsun]]

Are those Mochitest on "B2G ICS Emulator Debug" mandatory to be passed before check-in?

Comment 19

[Carsten Book [:Tomcat]]

(In reply to Bruce Sun [:brsun] from comment #18)
> Are those Mochitest on "B2G ICS Emulator Debug" mandatory to be passed
> before check-in?

well i think the general answer is that new checkins shall not regress test like causing test failures. Thats why we request try runs for checkin needed bugs. So i guess in this case yes tests have to be passed if test failures are caused by this checkin

Comment 20

[Ryan VanderMeulen [:RyanVM]]

OK, there was a bit of a misunderstanding here I think. Assuming the push to Try was on top of 1.3t, the debug mochitest failures aren't unexpected and are safe to ignore (might as well not run them in the future to save resources, fwiw).

However, it's also useful to point out that a patch is 1.3t-only to make it clear that this isn't landing on trunk/master or any other release branches since our standard branch landing policies say it has to land on master first (https://wiki.mozilla.org/Release_Management/B2G_Landing). It doesn't affect any other branches like 1.4 (Dolphin), right? :)

Please confirm my question above and we can go ahead and land this on 1.3t.

Comment 21

[Bruce Sun [:brsun]]

Thanks Ryan, and sorry that I did not deliver a clear message.

Yes, this patch should be committed onto 1.3t branch only. No other branches should be affected.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Thanks for the clarification, Bruce.

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/0a1750bee190