Closed
Bug 859711
Opened 11 years ago
Closed 11 years ago
[DeviceStorage/Audio/Video] The Filter Types in DeviceStorage doesn't be all allowed by ExternalHelperAppService.
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18+ verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 verified)
People
(Reporter: mchen, Assigned: mchen)
References
Details
Attachments
(5 files, 3 obsolete files)
The filter types in DeviceStorage is listed at http://mxr.mozilla.org/mozilla-central/source/toolkit/content/devicestorage.properties The extention name to MineType is listed at http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#387 http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#455 Some types in DeviceStorage are not supported by ExternalHelperAppService yet, so these files will be listed into music app (app parses meta data succesfully then allow to play) but can't really to play by audio tag. Ex: m4r, m4p, m4a (in m-c but b2g18), ogx, aac. So either we modify DeviceStorage to types we needs or add unsupported type into minetype list.
Assignee | ||
Comment 1•11 years ago
|
||
Oh, the owner of device storage would be Doug Tuner, so add Doug into cc list.
Assignee | ||
Comment 2•11 years ago
|
||
Hi Sandip, Does any document list what audio/video extension name we suppport on b2g1.1 and b2g 1.0.1? Then we can sync these between uriloader and devicestorage. Thanks.
Flags: needinfo?(skamat)
Comment 4•11 years ago
|
||
I think nsMediaSniffer should be changed also to check more mimetype. It returns "application/x-javascript-config" mimetype from those files, when I try to play the contents. (So....MediaElement cannot create proper player)
blocking-b2g: --- → leo?
Assignee | ||
Comment 5•11 years ago
|
||
Hi Chris, The list as below is the types filtered by DeviceStroage and some extension name are not supported by Gecko now. So may I know what is the supporting extension name from Gecko's point of view? Then we can decide remove them from filter type or add them into Gecko. Thanks. Ex: m4r, m4p, m4a (in m-c but b2g18), ogx, aac are not supported by gecko now. 1 # Extensions we recognize for DeviceStorage storage areas 2 pictures=*.jpe; *.jpg; *.jpeg; *.gif; *.png; *.bmp; 3 music=*.mp3; *.ogg; *.m4a; *.m4b; *.m4p; *.m4r; *.3gp; *.mp4; *.aac; *.m3u; *.pls; *.opus; 4 videos=*.mp4; *.mpeg; *.mpg; *.ogv; *.ogx; *.webm; *.3gp; *.ogg;
Flags: needinfo?(chris.double)
Comment 6•11 years ago
|
||
What's the user-facing result here? Do we communicate the "this type is unsupported" or "can't be played" message? If so, this would not be a blocker but a future enhancement/improvement.
Keywords: qawanted
Comment 7•11 years ago
|
||
The problem is that music list up some of those types but it cannot be played. (The music application freeze) Need to select one. 1. Do not list up those files that have problem. 2. Add new mimetype and support sniffing for those files.
Assignee | ||
Comment 8•11 years ago
|
||
Hi Leo, Could you provide some sample file here for testing? Thanks.
Assignee | ||
Comment 9•11 years ago
|
||
The problem of this attach file is that nsMediaSniffer [1] used a keyword "mp4" to match the field of "compatible_brands" in mp4 header and this will be a key to recognize whether the MimeType of this file is video/mp4 or not. But actually there can be three types of "compatible_brands" at least. 1. ismo 2. iso2 3. mp41 So maybe we can fix it in nsMediaSniffer or [2] in m-c already add extension name of m4a into extraMimeEntries then we just asked for uplifting ti. [1] http://mxr.mozilla.org/mozilla-central/source/toolkit/components/mediasniffer/nsMediaSniffer.cpp#61 [2] http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#509
Comment 10•11 years ago
|
||
Comment 11•11 years ago
|
||
Comment 12•11 years ago
|
||
(In reply to Marco Chen [:mchen] from comment #9) > Created attachment 741723 [details] > M4A file can be listed but be playable > > The problem of this attach file is that nsMediaSniffer [1] used a keyword > "mp4" to match the field of "compatible_brands" in mp4 header and this will > be a key to recognize whether the MimeType of this file is video/mp4 or not. > > But actually there can be three types of "compatible_brands" at least. > 1. ismo > 2. iso2 > 3. mp41 > > So maybe we can fix it in nsMediaSniffer or [2] in m-c already add extension > name of m4a into extraMimeEntries then we just asked for uplifting ti. > > [1] > http://mxr.mozilla.org/mozilla-central/source/toolkit/components/ > mediasniffer/nsMediaSniffer.cpp#61 > > [2] > http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/ > nsExternalHelperAppService.cpp#509 Right. As I told in Bug 865114, m4a file problem is simply solved by add routine like this in MedisSniffer. // The string "m4a". if (aData[4*i] == 0x4D && aData[4*i+1] == 0x34 && aData[4*i+2] == 0x41) { return MATCH_M4A; } when it return MATCH_M4A, in GetMIMETypeFromContent() if (res == MATCH_M4A) { aSniffedType.AssignLiteral(AUDIO_MP4); return NS_OK; } I already checked it's working. But I'm not sure about other types. (Other types are not listed up in Music app because of its metadata filtering)
Comment 13•11 years ago
|
||
(In reply to Marco Chen [:mchen] from comment #8) > Hi Leo, > > Could you provide some sample file here for testing? > > Thanks. I have three kind of test clip among them. m4a is already attached by you and I uploaded other two.
Assignee | ||
Comment 14•11 years ago
|
||
Hi Leo, I can't see attached aac or m4v files be listed in music app. And I found that they are filtered by music app. May I confirm with you?
Comment 15•11 years ago
|
||
The discussion above answers the qawanted request, although I'll note this was an inappropriate case for using qawanted - the reporter is a better person to ask in this case.
Keywords: qawanted
Comment 16•11 years ago
|
||
(In reply to Marco Chen [:mchen] from comment #14) > Hi Leo, > > I can't see attached aac or m4v files be listed in music app. And I found > that they are filtered by music app. May I confirm with you? Yes, right. Music cannot parse other file format. But..it's possible to enable it by adding sniff function on nsMediaSniffer. Moreover, other mimetype like mkv, ts, k3g, etc that can be parsed by android extractor, also can be played. I already tested it. The coverage of extension is up to you. I want just one rule. The files listed up by app, should be played well if you don't want any notification popup like "Cannot play this contents".
Updated•11 years ago
|
blocking-b2g: leo? → tef?
Updated•11 years ago
|
Whiteboard: [tef-triage]
Comment 17•11 years ago
|
||
Our understanding is that this is a request to support new extensions which were not planned to be supported in leo. (see comment 5 for supported formats) We will not block the release on the increased scope.
Comment 18•11 years ago
|
||
(In reply to Peter Dolanjski [:pdol] from comment #17) > Our understanding is that this is a request to support new extensions which > were not planned to be supported in leo. (see comment 5 for supported > formats) > We will not block the release on the increased scope. leo triage disagrees with this analysis. m4a files to my understanding was in scope and I believe was working previously. And the end result is that the files are showing up being unable to be played. Renom. I want the list of supported of formats by Sandip to be able to clarify this, cause there's missing information to make the call here.
blocking-b2g: - → tef?
Flags: needinfo?(skamat)
Comment 19•11 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #18) > m4a files to my understanding was > in scope and I believe was working previously. And the end result is that > the files are showing up being unable to be played. Renom. You're right, Jason. I misread based on the comment about mkv, ts, k3g formats. I'll let Sandip chime in on supported formats. Thanks for catching.
Assignee | ||
Comment 20•11 years ago
|
||
Attachment #744567 -
Flags: review?(chris.double)
Comment 21•11 years ago
|
||
Comment on attachment 744567 [details] [diff] [review] Support mp4 on isom&iso2 Paul's the media sniffer guru, I defer to him.
Attachment #744567 -
Flags: review?(chris.double) → review?(paul)
Flags: needinfo?(chris.double)
Comment 22•11 years ago
|
||
(In reply to Marco Chen [:mchen] from comment #5) > The list as below is the types filtered by DeviceStroage and some extension > name are not supported by Gecko now. So may I know what is the supporting > extension name from Gecko's point of view? Then we can decide remove them > from filter type or add them into Gecko. Thanks. > > Ex: m4r, m4p, m4a (in m-c but b2g18), ogx, aac are not supported by gecko > now. > > 1 # Extensions we recognize for DeviceStorage storage areas > 2 pictures=*.jpe; *.jpg; *.jpeg; *.gif; *.png; *.bmp; > 3 music=*.mp3; *.ogg; *.m4a; *.m4b; *.m4p; *.m4r; *.3gp; *.mp4; *.aac; > *.m3u; *.pls; *.opus; > 4 videos=*.mp4; *.mpeg; *.mpg; *.ogv; *.ogx; *.webm; *.3gp; *.ogg; I can only comment on the ones I was involved with supporting. .ogv, .webm, .ogg, mp4 (on some platforms), .mp3 (on some platforms), .wav, .oga. I'm not aware of, or sure of, what image formats or other music formats it supports.
Comment 23•11 years ago
|
||
Comment on attachment 744567 [details] [diff] [review] Support mp4 on isom&iso2 Review of attachment 744567 [details] [diff] [review]: ----------------------------------------------------------------- We should just implement the last version of the spec [1], which is more robust than randomly adding major_brands, and would fix the issue just as well. [1]: http://mimesniff.spec.whatwg.org/#signature-for-mp4
Attachment #744567 -
Flags: review?(paul) → review-
Updated•11 years ago
|
blocking-b2g: tef? → tef+
Comment 24•11 years ago
|
||
Seems like Marco's working on this so assigning to him. Please de/re-assign if this is incorrect.
Assignee: nobody → mchen
Assignee | ||
Comment 25•11 years ago
|
||
(In reply to Paul Adenot (:padenot) from comment #23) > Comment on attachment 744567 [details] [diff] [review] > Support mp4 on isom&iso2 > > Review of attachment 744567 [details] [diff] [review]: > ----------------------------------------------------------------- > > We should just implement the last version of the spec [1], which is more > robust than randomly adding major_brands, and would fix the issue just as > well. > > [1]: http://mimesniff.spec.whatwg.org/#signature-for-mp4 Hi Paul, > We should just implement the last version of the spec [1], which is more robust than > randomly adding major_brands, and would fix the issue just as well. I am not adding to filter major_brands but compatible_brand 1. I saw the code on nsMediaSniffer for mp4 is based on spec you said already. 2. Please refer to the attach file - "M4A file can be listed but be playable" and it's info is listed as below Major_verion: M4A Minor_version: null Compatible_Brand: isom & iso2. There is no any signature called mp4, so this file can not be recognized by sniffer now and that is why I post this patch.
Flags: needinfo?(paul)
Assignee | ||
Comment 26•11 years ago
|
||
On gaia side, the metadata.js from gaia/app/music/js used main_version in ftyp to recognize MP4 files. And it accepts M4A, M4B, mp42. I think we should sync sniffer between Gaia & Gecko or there will be an issue of Gaia can list files in playlist but Gecko can't play it.
Assignee | ||
Comment 27•11 years ago
|
||
Hi Chris, To info you again, and thanks for the reply in advance. One of the attached file format from Leo is XXX.aac. As I knew we can support audio track with AAC encoding format on m4a extension name but we don't support it on aac extension name. Could you help to confirm this?
Flags: needinfo?(chris.double)
Comment 28•11 years ago
|
||
(In reply to Marco Chen [:mchen] from comment #27) > Hi Chris, > > To info you again, and thanks for the reply in advance. > > One of the attached file format from Leo is XXX.aac. As I knew we can > support audio track with AAC encoding format on m4a extension name but we > don't support it on aac extension name. Could you help to confirm this? As far as I know that is the case but I don't know what changes have been made by B2G to support formats specific to that platform.
Flags: needinfo?(chris.double)
Comment 29•11 years ago
|
||
Comment on attachment 744567 [details] [diff] [review] Support mp4 on isom&iso2 Review of attachment 744567 [details] [diff] [review]: ----------------------------------------------------------------- Based on IRC discussion with mchen, we are landing this, but I filed bug 868333 to implement a more future proof solution.
Attachment #744567 -
Flags: review- → review+
Flags: needinfo?(paul)
Assignee | ||
Comment 30•11 years ago
|
||
This is a patch for adding m4a extension name into extraMimeEntries for mapping to video/mp4. This is used when nsMediaSniffer can't recognize some kind of mp4 file. Because this is added into m-c by bug 799315, here just extract what we need then porting to b2g18 branch.
Attachment #745076 -
Flags: review?(paul)
Comment 31•11 years ago
|
||
Comment on attachment 745076 [details] [diff] [review] Patch for b2g18 : Add m4a to extraMimeType Review of attachment 745076 [details] [diff] [review]: ----------------------------------------------------------------- I guess your commit message should sqy "Add mapping of between m4a extension name and audio/mp4 in extraMimeEntries for b2g18 branch." instead of video/mp4.
Attachment #745076 -
Flags: review?(paul) → review+
Assignee | ||
Comment 32•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=6093c684ed85
Assignee | ||
Comment 33•11 years ago
|
||
Add reviewer name and approval reason.
Attachment #744567 -
Attachment is obsolete: true
Attachment #746193 -
Flags: review+
Assignee | ||
Comment 34•11 years ago
|
||
Add reviewer name and approval reason.
Attachment #745076 -
Attachment is obsolete: true
Attachment #746194 -
Flags: review+
Assignee | ||
Comment 35•11 years ago
|
||
Lost to add the define of AUDIO_MP4 so add it into patch.
Attachment #746194 -
Attachment is obsolete: true
Attachment #746262 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Attachment #746262 -
Attachment is patch: true
Assignee | ||
Comment 36•11 years ago
|
||
Hi Leo, 1. The patch here will solve the issue on m4a file. 2. FxOS doesn't plan to support aac extension file name. 3. m4v support will be added by https://bugzilla.mozilla.org/show_bug.cgi?id=847006#c19 So after landing the patch, this bug will be closed.
Flags: needinfo?(leo.bugzilla.gecko)
Keywords: checkin-needed
Comment 37•11 years ago
|
||
https://hg.mozilla.org/projects/birch/rev/a4b5223f39b7
Keywords: checkin-needed
Comment 38•11 years ago
|
||
Given that we're heading to land, not going to worry about the needinfo request anymore.
Flags: needinfo?(skamat)
Comment 39•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a4b5223f39b7
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 40•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/fb62a95065da https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/41b7ed10abd5
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → fixed
status-firefox21:
--- → wontfix
status-firefox22:
--- → wontfix
status-firefox23:
--- → fixed
Updated•11 years ago
|
Flags: needinfo?(leo.bugzilla.gecko)
Comment 41•11 years ago
|
||
This bug is no longer reproduces As per comment 36 The attached mp4a file in visible and playable in "Music" app Environmental Variables: Inari Build ID: 20130531070208 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/be5c2ee11d02 Gaia: e7114bdf4078274fc127a3b2a58dad91d6884219 Environmental Variables: Unagi Build ID: 20130531070205 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/4f318822e72c Gaia: e1c59baed29e4665d1da9392f86239272073f07a
You need to log in
before you can comment on or make changes to this bug.
Description
•