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)
Tracking
()
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)
(deleted),
application/zip
|
Details | |
(deleted),
patch
|
brsun
:
review+
|
Details | Diff | Splinter Review |
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•10 years ago
|
||
We do support AMR formats..
Dominic, Can you take a look at this issue?
Flags: needinfo?(dkuo)
Comment 2•10 years ago
|
||
It caused by bug 1002897 and bug 1010434.
Updated•10 years ago
|
status-b2g-v1.3T:
--- → affected
Comment 3•10 years ago
|
||
(In reply to James Zhang (Spreadtrum) from comment #2)
> It caused by bug 1002897 and bug 1010434.
Jingmei, right?
Reporter | ||
Comment 4•10 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.
Updated•10 years ago
|
Flags: needinfo?(kkuo)
Comment 5•10 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•10 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)
Updated•10 years ago
|
Whiteboard: [partner-blocker][sprd319601]
Comment 8•10 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•10 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•10 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•10 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].
Comment 12•10 years ago
|
||
(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•10 years ago
|
||
Merge the patch from bug 1010685 to 1.3t.
Attachment #8454240 -
Flags: review?(roc)
Comment 14•10 years ago
|
||
(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 -
Flags: review?(roc) → review+
Assignee | ||
Comment 15•10 years ago
|
||
TBPL results: https://tbpl.mozilla.org/?tree=Try&rev=42a274002252
Attachment #8454240 -
Attachment is obsolete: true
Attachment #8454279 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 16•10 years ago
|
||
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•10 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•10 years ago
|
||
Are those Mochitest on "B2G ICS Emulator Debug" mandatory to be passed before check-in?
Flags: needinfo?(cbook)
Comment 19•10 years ago
|
||
(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)
Comment 20•10 years ago
|
||
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•10 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
Comment 22•10 years ago
|
||
Thanks for the clarification, Bruce.
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/0a1750bee190
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v1.3:
--- → unaffected
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.
Description
•