Closed
Bug 882683
Opened 11 years ago
Closed 11 years ago
[Audio] AAC audio format in .3gp container is not playing
Categories
(Firefox OS Graveyard :: Gaia::Music, defect, P1)
Tracking
(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)
People
(Reporter: mbarone976, Assigned: dkuo)
References
Details
(Whiteboard: [cr 488549], [TD-53529], [LeoVB+])
Attachments
(4 files)
STR 1. Open Music app and load an AAC audio format in 3gp container ACTUAL RESULT The media file is not played EXPECTED RESULT The media file should be played
Comment 1•11 years ago
|
||
Can we get a test file & test page here to demonstrate the bug? I believe this should work, but I need to see the file here under question.
Flags: needinfo?(mbarone976)
Comment 3•11 years ago
|
||
Works fine in the browser, so I'm guessing this is a bug in the music app not parsing a supported media type. AAC I believe is supported media type for 1.01, but I'm putting needinfo Sandip to clarify if this multimedia file should be supported in the music app for 1.1.
blocking-b2g: --- → leo?
Flags: needinfo?(skamat)
Comment 4•11 years ago
|
||
Currently in V1.0.1, there is no constraint to prevent 3rd party apps and web pages to play AAC in mp4 and AMR in 3gp but with Bug 847809 we have restricted these on V1.1. With v1.1, AMR support will be with .amr extension file name (raw AMR format) (Bug 847809).
Flags: needinfo?(skamat)
Comment 5•11 years ago
|
||
(In reply to Sandip Kamat from comment #4) > Currently in V1.0.1, there is no constraint to prevent 3rd party apps and > web pages to play AAC in mp4 and AMR in 3gp but with Bug 847809 we have > restricted these on V1.1. With v1.1, AMR support will be with .amr extension > file name (raw AMR format) (Bug 847809). Are you suggesting this is expected behavior? Doesn't seem right to lose AAC audio playback in the music app in support of MMS audio (sounds like a regression), but triage may be wrong.
Comment 6•11 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #5) > (In reply to Sandip Kamat from comment #4) > > Currently in V1.0.1, there is no constraint to prevent 3rd party apps and > > web pages to play AAC in mp4 and AMR in 3gp but with Bug 847809 we have > > restricted these on V1.1. With v1.1, AMR support will be with .amr extension > > file name (raw AMR format) (Bug 847809). > > Are you suggesting this is expected behavior? Doesn't seem right to lose AAC > audio playback in the music app in support of MMS audio (sounds like a > regression), but triage may be wrong. To my understanding, bug 847809 & a followup bug in bug 876099 was intending to restrict playing of AMR media types. AAC I believe should still play in the music app, especially since we are allowing it to play on the open web right now per bug 876099.
Comment 7•11 years ago
|
||
Just noticed this is about AAC (and not AMR) in 3gp. Roc - Did the recent change impact AAC in 3gp as well?
Flags: needinfo?(robert)
Music app doesn't display AAC file with .aac extension . Looks like a regression. I used 1.1 latest gecko/gaia build .
Comment 9•11 years ago
|
||
(In reply to Tapas Kumar Kundu from comment #8) > Created attachment 762374 [details] > AAC Audio file > > Music app doesn't display AAC file with .aac extension . Looks like a > regression. > > I used 1.1 latest gecko/gaia build . Well, that just answered the question is this is a regression. Looks like we broken AAC somewhere. Definitely a blocker then.
Keywords: regression
(In reply to Tapas Kumar Kundu from comment #8) > Created attachment 762374 [details] > AAC Audio file > > Music app doesn't display AAC file with .aac extension . Looks like a > regression. This file doesn't appear to be AAC in an MP4 container. I don't know what container format it is, but we only advertise support for AAC in MP4 and 3GP containers.
Comment 11•11 years ago
|
||
Hi, In V1.1 we didn't support raw aac file with Audio Data Transport Stream (ADTS) format. We just support aac in 3gp or mp4 container, so extension name of aac didn't be a legal filter type in devicestorage.property.
Comment 12•11 years ago
|
||
Hi, About another file of aac in 3gp, this issue is that music app can't recognize the major/compatible brand names in that test file. The major/compatible brand names in that file are 3gp5 3gp5/isom We may need music app to modify the key words for filtering.
Flags: needinfo?(dkuo)
Assignee | ||
Comment 13•11 years ago
|
||
Since raw aac file is not supported and devicestorage.properties doesn't allow *.aac so this is not a regression. We do support aac in *.3gp or *.mp4 container so as roc and Marco mentioned in comment 10 and comment 11, we will only fix the part with *.3gp or *.mp4 container.
Flags: needinfo?(dkuo)
Keywords: regression
Assignee | ||
Comment 14•11 years ago
|
||
The mp4 ftyp is "3gp5" which the metadata parser of Music app does not recognize yet, we should be able to fix this by modifying metadata.js in Music app, adding "3gp5" to the MP4Types object.
Comment 15•11 years ago
|
||
(In reply to Dominic Kuo [:dkuo] from comment #14) > Music app, adding "3gp5" to the MP4Types object. Hi Dominic, Please refer to link as below. http://en.wikipedia.org/wiki/ISO_base_media_file_format I would suggest that to add isom as the keyword in metadata parser for identifying mp4 or 3gp container. Because 1. 3gp5 is a main brand field in ftyp box and there are many variations. 2. both of mp4 and 3gp are all based on iso based media file format so they should all have isom in their compatible brand name in ftyp box.
Comment 16•11 years ago
|
||
If this isn't a regression, we probably can't block on this, right? Renom if someone feels differently here.
blocking-b2g: leo? → ---
Updated•11 years ago
|
Whiteboard: [cr 488549]
Reporter | ||
Comment 17•11 years ago
|
||
I want block on this as mentioned on comment #10, and comment #11.
blocking-b2g: --- → leo?
Reporter | ||
Comment 18•11 years ago
|
||
I want block on this as mentioned on comment #10, and comment #11.
Comment 19•11 years ago
|
||
Comment 10 suggests this is expected behavior. roc - please renom if we got that wrong.
blocking-b2g: leo? → -
Comments 8 was irrelevant to this bug and comments 9-11 were just elucidating that. This bug is valid and should stay open, and possibly block.
blocking-b2g: - → leo?
Flags: needinfo?(robert)
Comment 21•11 years ago
|
||
Triage- Partners agree to block on this per comment 20 and the rest of the discussions. Dominic, are you the right person to work on this?
blocking-b2g: leo? → leo+
Flags: needinfo?(dkuo)
Assignee | ||
Comment 22•11 years ago
|
||
Bug 882099 should also solve this issue and Jim is working on that, dependcency is also set.
Flags: needinfo?(dkuo)
Comment 23•11 years ago
|
||
Hi Dominic, After applying the patch from bug # 882099, iam still not able to scan this file(Content-01_AAC_Marvin_12.2kbps.3gp)
Assignee | ||
Comment 24•11 years ago
|
||
Leo, you probabaly need to "make reset-gaia" after you apply that patch, because your test file was already recorded as "unsupported" in MediaDB if you had launched Music before, or you can try to rename your test file and that should also trigger MediaDB to rescan it.
Comment 25•11 years ago
|
||
Hi Dominic, Still issue reproduced after a gaia reset. test file used : Content-01_AAC_Marvin_12.2kbps.3gp
Assignee | ||
Comment 26•11 years ago
|
||
Thanks Leo, I also reproduced this issue after apply the patch of bug 882099, and I am trying to figure out what's missing in that patch.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → dkuo
Can anyone confirm me whether https://bugzilla.mozilla.org/show_bug.cgi?id=882099 fixes this problem ?
Flags: needinfo?(dkuo)
Comment 28•11 years ago
|
||
Hi Tapas, Bugzilla ID :882099 fixes only mp4. With this patch , the 3gp still doesn't work.
Assignee | ||
Comment 29•11 years ago
|
||
Thanks Leo, there is another bug in the metadata parser of music app which caused this issue and I have to give one more patch for this bug.
Flags: needinfo?(dkuo)
Comment 30•11 years ago
|
||
Thanks Dominic. When do you expect to have that patch ready?
Flags: needinfo?(dkuo)
Assignee | ||
Comment 31•11 years ago
|
||
(In reply to Dietrich Ayala (:dietrich) from comment #30) > Thanks Dominic. When do you expect to have that patch ready? This bug took me a couple days because I have to figure out the structures of MP4 and 3GP containers from the specifications in detail, so that the metadata parser can handle the common files in the real life. And fortunately I just finished it and I am attaching it.
Flags: needinfo?(dkuo)
Assignee | ||
Comment 32•11 years ago
|
||
Jim, Since you are familiar with the metadata parser and David is PTO, I think this patch should go to you. The patch fixed the issues that some 3gp files cannot be recognized by the metadata parser, there are two major issues: 1. There is a logic error in shared/js/blobview.js that breaks the metadata parser, please see details in my comment. 2. It's possible that a 3gp file contains multiple tracks(odsm/sdsm/vide/soun/hint...), but some of the tracks are not sound track, we should only detect sound track for music app and it also contains the codec that we can identify is supported or not, so my patch added the part of checking Media Header Box for "vmhd" and "smhd", which indicates "Video Media Header Box" and "Sound Media Header Box". I have also added some comments about the specifications in the patch, and if you have any question, feel free to let me know, thanks.
Attachment #773845 -
Flags: review?(squibblyflabbetydoo)
Comment 33•11 years ago
|
||
Please email if/when it appears this won't make it for 7/15.
Comment 34•11 years ago
|
||
I should be able to review this tomorrow; I was hoping to get to it today, but then my AC unit broke and I had to deal with that instead.
Comment 35•11 years ago
|
||
Comment on attachment 773845 [details] Check trak atom to filter iso base media files This works for me with the AAC test files I have, and the code looks good too! It doesn't parse the file from comment 8 (even if I rename the extension), but I'm not sure that we should be parsing it. If that's ok, then r=me.
Attachment #773845 -
Flags: review?(squibblyflabbetydoo) → review+
Assignee | ||
Comment 36•11 years ago
|
||
From comment 10 and comment 11, we just support aac in 3gp or mp4 container for v1.1, and the test file in comment 8 is a raw aac with .aac extension, so we don't really support it. And since this patch got r+ I am landing it.
Assignee | ||
Comment 37•11 years ago
|
||
Landed on master: 49349122b9f2fb7f3e8b04d46fc221d0168d6f4b
Status: NEW → RESOLVED
Closed: 11 years ago
Hardware: x86 → ARM
Resolution: --- → FIXED
Assignee | ||
Comment 38•11 years ago
|
||
Uplifted commit 49349122b9f2fb7f3e8b04d46fc221d0168d6f4b as: v1-train: d52ce2278dbc2b2916a7054a220b24fb41aefd6e
status-b2g18:
--- → fixed
Comment 39•11 years ago
|
||
(In reply to Dominic Kuo [:dkuo] from comment #36) > From comment 10 and comment 11, we just support aac in 3gp or mp4 container > for v1.1, and the test file in comment 8 is a raw aac with .aac extension, > so we don't really support it. And since this patch got r+ I am landing it. Thanks for the confirmation! I just wanted to make sure we didn't miss something accidentally.
Comment 40•11 years ago
|
||
It is possible to play the content: Content-01_AAC_Marvin_12.2kbps from Music app. Also it is possible to attach and send it in a MMS successfully. Verified with Unagi v1-train 07/15 build: Gecko-2b310a1 Gaia-d52ce22 ref ril
Status: RESOLVED → VERIFIED
Comment 41•11 years ago
|
||
v1.1.0hd: d52ce2278dbc2b2916a7054a220b24fb41aefd6e
status-b2g-v1.1hd:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•