Closed
Bug 788204
Opened 12 years ago
Closed 12 years ago
Gecko doesn't detect MIME type of MP3 files
Categories
(Core :: Audio/Video, defect, P1)
Tracking
()
People
(Reporter: iliu, Assigned: dhylands)
References
Details
(Whiteboard: [WebAPI:P0] [caf:blocking])
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
cpearce
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dhylands
:
review+
|
Details | Diff | Splinter Review |
STR
====
var player = new Audio();
player.src='style/ringtones/ALARM_progressive_dapple.mp3';
Actual result:
- Can not load the .mp3 media.
- Attached log:
E/GeckoConsole( 1444): [JavaScript Warning: "HTTP "Content-Type" of "application/x-unknown-content-type" is not supported. Load of media resource app://clock.gaiamobile.org/style/ringtones/ALARM_progressive_dapple.mp3 failed." {file: "app://clock.gaiamobile.org/onring.html" line: 0}]
Expected result:
- It should be work.
Updated•12 years ago
|
blocking-basecamp: --- → ?
Component: General → Video/Audio
Product: Boot2Gecko → Core
Reporter | ||
Updated•12 years ago
|
OS: Other → Gonk
This is odd ... if I go to http://people.mozilla.com/~cjones/media.html in the browser app and tap "MP3", I get playback. Will see if I can make a testcase that doesn't work.
I added two new options to http://people.mozilla.com/~cjones/media.html to directly play mp3/ogg using an Audio element directly, and I still don't reproduce the bug. Playback works just fine.
Ian, can you make a test case using the "Template" app that reproduces your bug? Were you testing with a desktop build?
Comment 3•12 years ago
|
||
The issue is the wrong mime-type as the error message already indicates "application/x-unknown-content-type". a wrong content-type is intended to not work.
The testcase as http://people.mozilla.com/~cjones/media.html uses this URL:
http://pianoadventures.com/audio/mp3s/CD1002-21.mp3 and the server sends this with Content-Type: audio/mpeg according to http://www.web-sniffer.net
Ah, this is gecko not detecting the mp3 MIME type properly. I saw that we weren't doing this when working on a previous bug, and started to patch that, but tossed out the WIP since the "real bug" lay elsewhere.
blocking-basecamp: ? → +
Summary: Can not playback when audio tag uses a MP3 as src → Gecko doesn't detect MIME type of MP3 files
Comment 5•12 years ago
|
||
i am pretty sure there are other mimetypes that we should support but don't.
Comment 7•12 years ago
|
||
The patches in bug 567077 will hopefully sniff MP3 and thus fix this bug.
The way I had started cargo-culting this in is by
- following the conditional recognition of mp4 when gstream support is enabled
- add a check for media plugins
- add mp3 to the list of recognized extensions
(In reply to Chris Pearce (:cpearce) from comment #7)
> The patches in bug 567077 will hopefully sniff MP3 and thus fix this bug.
Note, this bug appears to be triggered when loading a .mp3 file off of disk: we don't detect audio/mpeg. Doing the same thing with .ogg results in OGG being detected. Will bug 567077 fix that?
Comment 10•12 years ago
|
||
Ah. We need to add audio/mpeg to nsMimeTypes.h and nsExternalHelperAppService.cpp, like we do for audio/ogg and friends.
OK --- that's the code I was cargo-culting in comment 8.
dhylands, wan to grab this?
Reporter | ||
Comment 12•12 years ago
|
||
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> I added two new options to http://people.mozilla.com/~cjones/media.html to
> directly play mp3/ogg using an Audio element directly, and I still don't
> reproduce the bug. Playback works just fine.
>
> Ian, can you make a test case using the "Template" app that reproduces your
> bug? Were you testing with a desktop build?
Yes, It doesn't work with a desktop build.
blocking-basecamp: + → ?
Updated•12 years ago
|
blocking-basecamp: ? → +
Updated•12 years ago
|
Whiteboard: [WebAPI:P0]
Updated•12 years ago
|
Assignee: nobody → dhylands
Assignee | ||
Comment 13•12 years ago
|
||
Attachment #660283 -
Flags: review?(cpearce)
Updated•12 years ago
|
Attachment #660283 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 14•12 years ago
|
||
Looking at frameworks/base/media/libstagefright/MediaExtractor.cpp, it can deal with the following mime types:
MEDIA_MIMETYPE_CONTAINER_MPEG4
MEDIA_MIMETYPE_AUDIO_MPEG
MEDIA_MIMETYPE_AUDIO_AMR_NB audio/3gpp
MEDIA_MIMETYPE_AUDIO_AMR_WB audio/amr-wb
MEDIA_MIMETYPE_AUDIO_FLAC audio/flac
MEDIA_MIMETYPE_CONTAINER_WAV audio/wav
MEDIA_MIMETYPE_CONTAINER_OGG application/ogg
MEDIA_MIMETYPE_CONTAINER_MATROSKA video/x-matroska
MEDIA_MIMETYPE_CONTAINER_MPEG2TS video/mp2ts
MEDIA_MIMETYPE_CONTAINER_WVM video/wvm
MEDIA_MIMETYPE_AUDIO_AAC_ADTS audio/aac-adts
MEDIA_MIMETYPE_CONTAINER_MPEG2PS video/mp2p
I'm not which, if any of these, we also want to add.
Comment 15•12 years ago
|
||
In my opinion, we should add none of those.
We don't want to encourage format inflation, we want authors to be able to target the smallest set of codecs/containers to get the widest possible coverage across all browsers.
Assignee | ||
Comment 16•12 years ago
|
||
Being a good boy and submitted to try first:
https://tbpl.mozilla.org/?tree=Try&rev=5021785d573b
Comment 17•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Reporter | ||
Comment 18•12 years ago
|
||
I got some error log with "unknow-content-type".
===== log =====
E/GeckoConsole( 5538): [JavaScript Warning: "HTTP "Content-Type" of "application/x-unknown-content-type" is not supported. Load of media resource app://clock.gaiamobile.org/style/ringtones/ALARM_progressive_dapple.mp3 failed." {file: "app://clock.gaiamobile.org/onring.html" line: 0}]
Gaia: 12a76c897aea24bdb3dee1cad0eb090bd3ffa2d2
Gecko: add440caf2f26a55f86e456502689709a9fed143
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 19•12 years ago
|
||
Hi dhylands :)
Could you please revisit this issue? According to Ian's investigation, our Alarm/Music Apps on B2G doesn't work after this change.
Comment 20•12 years ago
|
||
Reading files from a package always results in application/x-unknown-content-type I think. I ran into the same problem with video. We could fix this in the reader code I guess, or try to guess the MP3 mime type from the file name.
Comment 21•12 years ago
|
||
Jonas, what do you think?
(In reply to Andreas Gal :gal from comment #20)
> Reading files from a package always results in
> application/x-unknown-content-type I think. I ran into the same problem with
> video. We could fix this in the reader code I guess, or try to guess the MP3
> mime type from the file name.
If it returned application/octet-stream, we would sniff the file to find out the type.
I think we should add application/x-unknown-content-type to the sniff list. Edwin can do this.
Assignee: dhylands → eflores
Nothing wrong with the approach here; only issue is that we no longer use media plugins for MP3 on B2G.
Attachment #675416 -
Flags: review?(dhylands)
Assignee | ||
Comment 24•12 years ago
|
||
Comment on attachment 675416 [details] [diff] [review]
Fix ifdef
Review of attachment 675416 [details] [diff] [review]:
-----------------------------------------------------------------
::: uriloader/exthandler/nsExternalHelperAppService.cpp
@@ +481,5 @@
> { AUDIO_WEBM, "webm", "Web Media Audio" },
> #ifdef MOZ_DASH
> { APPLICATION_DASH, "mpd", "DASH Media Presentation Description" },
> #endif
> +#if defined(MOZ_MEDIA_PLUGINS) or defined(MOZ_WIDGET_GONK)
I've never seen the word "or" used in this context. I've always seen ||.
Just to convince myself, I tried with g++/gcc 4.6 and it seems to work.
I'm concerned about compatibility though. Do you know when the word "or" was allowed?
I'm going to say that you should use || (unless you can convince me that this is supported in all of the compilers that we care about)
Attachment #675416 -
Flags: review?(dhylands) → review-
Assignee | ||
Comment 25•12 years ago
|
||
r=me with or changed to || (so you don't have to re-review)
Updated•12 years ago
|
Whiteboard: [WebAPI:P0] → [WebAPI:P0] [caf:blocking]
Comment 26•12 years ago
|
||
dhylands, can you please take this bug and land it with your changes?
Thanks.
Updated•12 years ago
|
Assignee: eflores → dhylands
Assignee | ||
Comment 27•12 years ago
|
||
My changes were landed a while ago, but I'm happy to land the new fix.
Assignee | ||
Comment 28•12 years ago
|
||
Pushed to inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/47b45ff8ef0d
Assignee | ||
Comment 29•12 years ago
|
||
Updating the patch that was pushed for completeness
Attachment #675416 -
Attachment is obsolete: true
Attachment #678849 -
Flags: review+
Comment 30•12 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 31•12 years ago
|
||
Updated•12 years ago
|
status-firefox18:
--- → fixed
status-firefox19:
--- → fixed
Updated•12 years ago
|
Priority: -- → P1
Comment 32•12 years ago
|
||
Ignore move to P1, didn't realize it was already fixed.
You need to log in
before you can comment on or make changes to this bug.
Description
•