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)

58 Branch
defect

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)

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.
As a "positive" test case, this attached file (Magic_sfx_Arabic female_024_Opus_VBR.opus) loads fine in Firefox and Chrome.
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
Alfredo,
IIRC, you handled a similar bug before. Can you check it?
Flags: needinfo?(ayang)
Priority: -- → P3
Assignee: nobody → ayang
Flags: needinfo?(ayang)
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...
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)
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.
Attached file test.html
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: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attached patch media.patchSplinter Review
Attachment #8932045 - Flags: review?(jyavenard)
Does the "NS_" prefix in the media.patch file's variables stand for NetScape?!
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+
> 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.
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
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?
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
[Tracking Requested - why for this release]:
https://hg.mozilla.org/mozilla-central/rev/ba2f695d6075
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
(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'.
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+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: