Closed
Bug 1417113
Opened 7 years ago
Closed 7 years ago
NS_ERROR_DOM_MEDIA_METADATA_ERR (0x806e0006) error when loading Ogg Opus file from local drive
Categories
(Core :: Audio/Video: Playback, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | wontfix |
firefox58 | - | fixed |
firefox59 | --- | fixed |
People
(Reporter: oresteszoupanos, Assigned: baku)
References
Details
(Keywords: regression)
Attachments
(5 files)
2.23 KB,
audio/ogg
|
Details | |
5.62 KB,
audio/ogg
|
Details | |
18.00 KB,
audio/ogg
|
Details | |
1.20 KB,
text/html
|
Details | |
1.39 KB,
patch
|
jya
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171103003834
Steps to reproduce:
Tried loading the attached Ogg Opus file from my local drive into the below webpage.
HTML and JavaScript:
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
</head>
<body>
<p>
<input type="file" id="files" name="files[]" accept="audio/ogg,.ogg,.opus" multiple/>
</p>
<ol id="list"></ol>
<script>
//check if this browser supports various APIs we need
var us = "Unsupported APIs:";
if(!window.Blob){us += "\n - Blob";}
if(!window.File){us += "\n - File";}
if(!window.FileList){us += "\n - FileList";}
if(!window.FileReader){us += "\n - FileReader";}
if(!window.URL){us += "\n - URL";}
if(us != "Unsupported APIs:"){alert(us);}
function loadFiles(evnt) {
document.getElementById("list").innerHTML = ""; //clear previously loaded files
var files = evnt.target.files; //files is an array of File objects. List some properties
console.log("# of files selected: ",files.length);
//cycle through all files and add their details to the output tags
for(var i=0,f; f=files[i]; i++){
//save file to a URL and feed to audio tag source
var fileURL = window.URL.createObjectURL(f);
document.getElementById("list").innerHTML += ['<li><audio id="audio',i,'" controls src="',fileURL,'"></audio>'
,' - <strong>', f.name, '</strong> (', f.type || "n/a", ') - ', f.size.toLocaleString(), ' bytes.'
,'<div id="header',i,'"></div></li>'].join('');
}
}
document.getElementById("files").addEventListener("change", loadFiles, false);
</script>
</body>
</html>
Actual results:
Console reported the following error:
Media resource blob:http://localhost/fc601ecb-380e-4322-aa62-7e729c2096cd could not be decoded, error: Error Code: NS_ERROR_DOM_MEDIA_METADATA_ERR (0x806e0006)
Expected results:
It should have loaded a playable audio tag with the selected .opus file as a src attribute.
Tested in Chrome 61.0.3163.100 and it loaded it without a problem.
Reporter | ||
Comment 1•7 years ago
|
||
As a "positive" test case, this attached file (Magic_sfx_Arabic female_024_Opus_VBR.opus) loads fine in Firefox and Chrome.
Comment 2•7 years ago
|
||
The failing attachment plays fine when loaded over http (e.g. by clicking on the attachment link) but not when loaded from a local disk, either from the example html or directly into a video document. Something about the origin or access method is confusing the file.
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
Comment 3•7 years ago
|
||
Alfredo,
IIRC, you handled a similar bug before. Can you check it?
Flags: needinfo?(ayang)
Priority: -- → P3
Updated•7 years ago
|
Assignee: nobody → ayang
Flags: needinfo?(ayang)
Comment 4•7 years ago
|
||
It fails when loaded from test html page but ok when playing it directly.
[MediaPDecoder #1]: D/MediaDemuxer OggDemuxer(0x111dd4800)::ReadMetadata: OggDemuxer::ReadOggPage failed? leaving ReadMetadata...
Comment 5•7 years ago
|
||
It failed to read data when parsing header but it is ok when e10s is disabled.
[MediaPlayback #1]: D/MediaDecoder Decoder=0x11540a800 state=DECODING_METADATA Dispatching AsyncReadMetadata
[MediaPDecoder #1]: D/MediaDemuxer OggDemuxer(0x111334800)::ReadMetadata: OggDemuxer::ReadMetadata called!
[MediaPDecoder #1]: D/MediaResourceIndex 0x111334b50 ReadAt(4096@0) - length is 2282 (too short!), will fallback to blocking read as the caller requested...
[Parent 33246, IPDL Background] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80470002: file /Users/alfredo/mozilla/mozilla-unified/netwerk/base/nsFileStreams.cpp, line 74
[MediaPDecoder #1]: D/MediaResourceIndex 0x111334b50 ReadAt(4096@0) - fallback uncached read got 0 bytes -> NS_OK, 0
[MediaPDecoder #1]: D/MediaDemuxer OggDemuxer(0x111334800)::ReadMetadata: OggDemuxer::ReadOggPage failed? leaving ReadMetadata...
[Child 33261, MediaPlayback #1] WARNING: Decoder=11540a800 state=DECODING_METADATA Decode metadata failed, shutting down decoder: file /Users/alfredo/mozilla/mozilla-unified/dom/media/MediaDecoderStateMachine.cpp, line 372
[Child 33261, MediaPlayback #1] WARNING: Decoder=11540a800 Decode error: NS_ERROR_DOM_MEDIA_METADATA_ERR (0x806e0006): file /Users/alfredo/mozilla/mozilla-unified/dom/media/MediaDecoderStateMachine.cpp, line 3351
Assignee: ayang → nobody
Flags: needinfo?(jwwang)
Reporter | ||
Comment 6•7 years ago
|
||
Hmm, weird that it says "length is 2282 (too short!)". I attach a bigger file which also throws the same error, in case you need another failing file, to help home in on the problem.
Comment 7•7 years ago
|
||
This is a test page to repro the issue in comment 0:
1. Click the "Browse..." button.
2. Select "- Magic_sfx_Arabic female_006_Opus_VBR.opus" from the local storage.
I got this error which happened in nsFileStreamBase::Tell().
[Parent 10084, IPDL Background] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80470002: file /home/jwwang/codebase/mozilla-central/firefox/netwerk/base/nsFileStreams.cpp, line 74
Hi baku,
CloneableWithRangeMediaResource is used to open the local file. Can you check this out?
Flags: needinfo?(jwwang) → needinfo?(amarchesini)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8932045 -
Flags: review?(jyavenard)
Reporter | ||
Comment 9•7 years ago
|
||
Does the "NS_" prefix in the media.patch file's variables stand for NetScape?!
Comment 10•7 years ago
|
||
Comment on attachment 8932045 [details] [diff] [review]
media.patch
Review of attachment 8932045 [details] [diff] [review]:
-----------------------------------------------------------------
Please amend the commit description. Having in the title what was done and in the body what was wrong and how it was fixed.
Also, seems to me that you're fixing two separate and unrelated issues in here...
shouldn't the patch be split in two?
Should ask to uplift this to 58
::: dom/media/CloneableWithRangeMediaResource.cpp
@@ +43,5 @@
> uint32_t done = 0;
> do {
> uint32_t read;
> nsresult rv = SyncRead(aBuffer + done, aSize - done, &read);
> + if (NS_SUCCEEDED(rv) && read == 0) {
could you ever get partial read with SynRead?
@@ +51,1 @@
> return rv;
shouldn't aRead be set to whatever has been read so far (if any) ?
Attachment #8932045 -
Flags: review?(jyavenard) → review+
Assignee | ||
Comment 11•7 years ago
|
||
> could you ever get partial read with SynRead?
It's not a 'partial' read. We call SyncRead with a size. If we read less than that, we still need to consider the reading a success.
> shouldn't aRead be set to whatever has been read so far (if any) ?
No. aRead value should not be used if we return an error.
I think that the 2 blocks in the patch are related. In theory, read() should not return NS_BASE_STREAM_CLOSED. Some nsIInputStreams do return such error, but we should not propagate outside SyncRead. Initially the patch was:
if ((rv == NS_BASE_STREAM_CLOSED || NS_SUCCEEDED(rv)) && read == 0) {
but just because we have our SyncRead(), let's handle with NS_BASE_STREAM_CLOSED in that method without propagating it.
Comment 12•7 years ago
|
||
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e946e79283a
CloneableWithRangeMediaResource InputStreamReader::Read must always return what has been read from the buffer, r=jya
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8932045 [details] [diff] [review]
media.patch
Approval Request Comment
[Feature/Bug causing the regression]: bug 1388125
[User impact if declined]: some media files are not correctly read.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: just landed. Tested locally.
[Needs manual test from QE? If yes, steps to reproduce]: yes, there is a test case.
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: The patch consists in a check of the return value of SyncRead(). Instead returning NS_OK, the patch updates aRead with the amount of data read from the stream so far.
[String changes made/needed]: none
Attachment #8932045 -
Flags: approval-mozilla-beta?
Comment 14•7 years ago
|
||
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba2f695d6075
CloneableWithRangeMediaResource InputStreamReader::Read must always return what has been read from the buffer, r=jya
Comment 15•7 years ago
|
||
[Tracking Requested - why for this release]:
Blocks: 1388125
status-firefox57:
--- → affected
status-firefox58:
--- → fix-optional
tracking-firefox58:
--- → ?
Keywords: regression
Comment 16•7 years ago
|
||
bugherder |
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
![]() |
||
Comment 17•7 years ago
|
||
bugherder |
Comment 18•7 years ago
|
||
(In reply to oresteszoupanos from comment #9)
> Does the "NS_" prefix in the media.patch file's variables stand for
> NetScape?!
Yes. the NS_ and ns prefixes abbreviate 'Netscape'.
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 19•7 years ago
|
||
Comment on attachment 8932045 [details] [diff] [review]
media.patch
Fix a video playback issue that some media files are not correctly read. Beta58+.
Attachment #8932045 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•7 years ago
|
Comment 20•7 years ago
|
||
bugherder uplift |
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•