Closed Bug 788204 Opened 8 years ago Closed 8 years ago

Gecko doesn't detect MIME type of MP3 files

Categories

(Core :: Audio/Video, defect, P1, major)

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED FIXED
mozilla18
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: iliu, Assigned: dhylands)

References

Details

(Whiteboard: [WebAPI:P0] [caf:blocking])

Attachments

(2 files, 1 obsolete file)

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.
blocking-basecamp: --- → ?
Component: General → Video/Audio
Product: Boot2Gecko → Core
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?
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
i am pretty sure there are other mimetypes that we should support but don't.
Duplicate of this bug: 788273
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?
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?
(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: + → ?
blocking-basecamp: ? → +
Whiteboard: [WebAPI:P0]
Assignee: nobody → dhylands
Attachment #660283 - Flags: review?(cpearce)
Attachment #660283 - Flags: review?(cpearce) → review+
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.
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.
Being a good boy and submitted to try first:
https://tbpl.mozilla.org/?tree=Try&rev=5021785d573b
https://hg.mozilla.org/mozilla-central/rev/0b4ad0fcea64
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
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 → ---
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.
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.
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
Attached patch Fix ifdef (obsolete) — Splinter Review
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)
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-
r=me with or changed to || (so you don't have to re-review)
Whiteboard: [WebAPI:P0] → [WebAPI:P0] [caf:blocking]
dhylands, can you please take this bug and land it with your changes?

Thanks.
Assignee: eflores → dhylands
My changes were landed a while ago, but I'm happy to land the new fix.
Attached patch Fix #ifdef v2Splinter Review
Updating the patch that was pushed for completeness
Attachment #675416 - Attachment is obsolete: true
Attachment #678849 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/47b45ff8ef0d
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Priority: -- → P1
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.