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

RESOLVED FIXED in Firefox OS v1.3T

Status

()

Core
Audio/Video
--
major
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jingmei.zhang, Assigned: brsun)

Tracking

unspecified
2.0 S6 (18july)
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(blocking-b2g:1.3T+, b2g-v1.3 unaffected, b2g-v1.3T fixed, b2g-v1.4 unaffected, b2g-v2.0 unaffected, b2g-v2.1 unaffected)

Details

(Whiteboard: [partner-blocker][sprd319601])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8448536 [details]
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

4 years ago
We do support AMR formats..
Dominic, Can you take a look at this issue?
Flags: needinfo?(dkuo)
It caused by bug 1002897 and bug 1010434.
status-b2g-v1.3T: --- → affected
(In reply to James Zhang (Spreadtrum) from comment #2)
> It caused by bug 1002897 and bug 1010434.

Jingmei, right?
(Reporter)

Comment 4

4 years ago
(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.
Flags: needinfo?(kkuo)

Comment 5

4 years ago
(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

4 years ago
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.
Flags: needinfo?(dkuo) → needinfo?(brsun)
broken feature, lets fix it.
blocking-b2g: 1.3T? → 1.3T+
Whiteboard: [partner-blocker][sprd319601]

Comment 8

4 years ago
Bruce/Eric,

Can you please help here (see comment 6)

Thanks
Hema
Component: Gaia::Music → Video/Audio
Flags: needinfo?(echou)
Product: Firefox OS → Core
(Assignee)

Comment 9

4 years ago
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
Flags: needinfo?(brsun)
(Assignee)

Comment 10

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

Comment 11

4 years ago
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].
(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.
Flags: needinfo?(wchang)
Flags: needinfo?(kkuo)
Flags: needinfo?(echou)
(Assignee)

Comment 13

4 years ago
Created attachment 8454240 [details] [diff] [review]
bug1032678_merge_bug1010685.patch

Merge the patch from bug 1010685 to 1.3t.
Attachment #8454240 - Flags: review?(roc)
(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.
Flags: needinfo?(wchang)
(Assignee)

Comment 15

4 years ago
Created attachment 8454279 [details] [diff] [review]
bug1032678_merge_bug1010685.checkin-needed.patch

TBPL results: https://tbpl.mozilla.org/?tree=Try&rev=42a274002252
Attachment #8454240 - Attachment is obsolete: true
Attachment #8454279 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Bruce: do you know if the test failure on debug from debug on this try run are related to this patch ?
Keywords: checkin-needed
(Assignee)

Comment 17

4 years ago
(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
(Assignee)

Comment 18

4 years ago
Are those Mochitest on "B2G ICS Emulator Debug" mandatory to be passed before check-in?
Flags: needinfo?(cbook)
(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
Flags: needinfo?(cbook)
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.
Assignee: nobody → brsun
Flags: needinfo?(brsun)
(Assignee)

Comment 21

4 years ago
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.
Flags: needinfo?(brsun)
Keywords: checkin-needed
Thanks for the clarification, Bruce.

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/0a1750bee190
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-b2g-v1.3: --- → unaffected
status-b2g-v1.3T: affected → fixed
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → unaffected
status-b2g-v2.1: --- → unaffected
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S6 (18july)
You need to log in before you can comment on or make changes to this bug.