Closed Bug 1032678 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Audio/Video, defect)

ARM
Gonk (Firefox OS)
defect
Not set
major

Tracking

()

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3 --- unaffected
b2g-v1.3T --- fixed
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.1 --- unaffected

People

(Reporter: jingmei.zhang, Assigned: brsun)

Details

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

Attachments

(2 files, 1 obsolete file)

Attached file armNB-and-armWB.zip (deleted) —
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.
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.
(In reply to James Zhang (Spreadtrum) from comment #2) > It caused by bug 1002897 and bug 1010434. Jingmei, right?
(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)
(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.
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]
Bruce/Eric, Can you please help here (see comment 6) Thanks Hema
Component: Gaia::Music → Video/Audio
Flags: needinfo?(echou)
Product: Firefox OS → Core
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)
Update: FileMediaResource::Tell() returns zero even when nsISeekableStream::Tell() returns NS_BASE_STREAM_CLOSED due to nsFileStreamBase::mFD is nullptr.
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)
Attached patch bug1032678_merge_bug1010685.patch (obsolete) (deleted) — Splinter Review
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)
Attachment #8454240 - Attachment is obsolete: true
Attachment #8454279 - Flags: review+
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
(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
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)
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
Status: NEW → RESOLVED
Closed: 10 years ago
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.

Attachment

General

Creator:
Created:
Updated:
Size: