WebM VP9 could not be decoded

RESOLVED FIXED in Firefox 46

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jlgrall, Assigned: nical)

Tracking

({regression})

43 Branch
mozilla46
regression
Points:
---

Firefox Tracking Flags

(firefox43 unaffected, firefox44+ unaffected, firefox45+ unaffected, firefox46+ fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151223140742

Steps to reproduce:

Go to: http://atelier-st-andre.net/tests/error-ff43-webm-vp9/test-case1.html




Actual results:

In Firefox 43.0.3 on Mac OS X 10.6.8.

The WebM VP9 video could not be decoded.
Also, when there is a second source, a mp4 h264 file, it is not loaded as a fallback.


Expected results:

In Firefox 42 it worked well and it still works well (I re-downloaded Firefox 42 to try it).
Component: Audio/Video → Audio/Video: Playback

Comment 1

2 years ago
Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9e4218058d633480b4a17db6cf3d3831418b6ec3&tochange=29f37fd4c13e4309b97e15a5879931819c8220e9

Nicolas Silva — Bug 1224254 - Don't try to allocate unreasonably large textures. r=Bas
(this bug has been backported into FF43 Beta)
Blocks: 1224254
Status: UNCONFIRMED → NEW
status-firefox43: --- → affected
status-firefox44: --- → affected
status-firefox45: --- → affected
status-firefox46: --- → affected
tracking-firefox44: --- → ?
tracking-firefox45: --- → ?
tracking-firefox46: --- → ?
Ever confirmed: true
Flags: needinfo?(nical.bugzilla)
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All

Comment 2

2 years ago
Created attachment 8703289 [details]
flaque2.webm

Comment 3

2 years ago
Is there a reason why no test triggered this regression?
I was just investigating this. I'm getting shmem allocation failure in one of the webm ref test when using the ffvp8 decoder (and in debug mode it triggers various asserts).
Regression, tracking it.
status-firefox43: affected → wontfix
tracking-firefox45: ? → +
tracking-firefox46: ? → +

Updated

2 years ago
tracking-firefox44: ? → +
(Assignee)

Comment 6

2 years ago
I got access to a mac and reproduced this locally. The reason this video is failing is that its cb and cr planes are slightly bugger than its y planes, in the texture allocation code there's a sanity check that the y size is superior or equal to the y size and I indeed added the check in that range. The good news is that it's trivial to revert this check and uplift the fix.
Not that it also triggers some warnings in VPXDecoder.cpp, so I don't know how robust we are against this kind of inputs.
Flags: needinfo?(nical.bugzilla)

Comment 7

2 years ago
I'm on Win 7 and it's reproducible too.
(Reporter)

Comment 8

2 years ago
Thanks for the infos @nical, it is good news.

Related question:
In the case that the video decoder considers the file to be corrupted, shouldn't Firefox try to play the next available source (ie. the .mp4 file) ? The second source is not played as you can see on the test page (http://atelier-st-andre.net/tests/error-ff43-webm-vp9/test-case1.html), even though Firefox has no problem playing it when it is the unique source.
OS: All → Unspecified
Hardware: All → Unspecified
(Assignee)

Comment 9

2 years ago
(In reply to jlgrall from comment #8)
> Related question:
> In the case that the video decoder considers the file to be corrupted,
> shouldn't Firefox try to play the next available source (ie. the .mp4 file)
> ?

That makes sense to me but I don't know that stuff well enough to answer how that interacts with this case. Here what actually happens is that we consider the stream to be valid "enough", then decode a frame, but fail to allocate the texture storage to copy the frame into and send to the screen. Looks like some DOM/media code sees that and comes back on the decision that the stream is valid (which is good) but maybe it doesn't do the fallback as it would have if it had found out the stream wasn't readable before actually decoding a frame. But I am just speculating, I am not a good person to answer this question.
Assignee: nobody → nical.bugzilla
(Assignee)

Comment 10

2 years ago
Created attachment 8704229 [details] [diff] [review]
allow cbcr planes that are bigger than the y plane (no matter how silly that is)

That'll have to be uplifted (if we care about weird videos like this test case to work)
Attachment #8704229 - Flags: review?(jmuizelaar)
(Reporter)

Comment 11

2 years ago
How I got this video with strange sizes ?
Maybe this little fact can explain it, though I am not sure, because I am a beginner at video conversion.

I saved the command line that generated these videos (a few months ago, I converted a batch of old QuickTime .mov videos). It was something like:
Local Settings\Application Data\dmMediaConverter\Config\ffmpeg.exe" -fflags +genpts -y -i "C:\videos\carnations5.mov" -map 0:0 -c:v:0 libvpx-vp9 -crf:v:0 10 -metadata:s:v:0 "rotate=179.897" -metadata:s:v:0 "creation_time=2004-10-26 22\:39\:14" -metadata:s:v:0 "language=eng" -metadata:s:v:0 "handler_name=Gestionnaire d�alias Apple" -metadata:s:v:0 "encoder=Vidéo" -threads:v:0 2 -shortest "C:\videos\carnations5.webm"

The rotate parameter looked strange, but it worked. That command line was automatically generated by dmMediaConverter (http://dmsimpleapps.blogspot.ch/2014/04/dmmediaconverter.html)

It was on Windows XP SP3, with dmMediaConverter 1.8.0 as a GUI to ffmpeg 2.8.1
(In reply to jlgrall from comment #8)
> Thanks for the infos @nical, it is good news.
> 
> Related question:
> In the case that the video decoder considers the file to be corrupted,
> shouldn't Firefox try to play the next available source (ie. the .mp4 file)
> ? The second source is not played as you can see on the test page
> (http://atelier-st-andre.net/tests/error-ff43-webm-vp9/test-case1.html),
> even though Firefox has no problem playing it when it is the unique source.

We have a big for that somewhere.
The fallback is only used if:
1- format is completely unsupported
2- file can't be accessed due to network problem.

Once it's been determined that we can do both, playback will start, and if there's a decoding error the video element will go into error.
By that time, our architecture doesn't allow restarting and trying another format.

Not that it would be possible nor make sense to do so anyway. What if the decoding error occurs much later, say halfway through the video. Would you then switch to the next file and seek to where we were in the previous file? What if then the two src show different content we can't detect that.

So while I agree it would make sense to fallback upon decoding error, as always everything is in the details.
Personally here, I wouldn't apply those changes: the video is invalid, that we used to play it was a bug :)
It's the media that needs fixing.
Or simply swap the two lines so h264 will be used first. It will make playback starts faster on IE, Edge and Safari

at this stage, only chrome supports webm, and they are known to allow any crap to play which isn't always good overall.
(Assignee)

Comment 14

2 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #13)
> Personally here, I wouldn't apply those changes: the video is invalid, that
> we used to play it was a bug :)
> It's the media that needs fixing.
> Or simply swap the two lines so h264 will be used first. It will make
> playback starts faster on IE, Edge and Safari
> 
> at this stage, only chrome supports webm, and they are known to allow any
> crap to play which isn't always good overall.

I am OK with not supporting this. I don't feel like I can personally make this decision, but since you are in the media crew I'm fine with whatever you think is adequate here.
How about we catch this a bit earlier in the decoding pipeline, though? The way this fails currently makes it look more like a bug in the texture allocation code than a conscious decision to not support weird yuv formats.

Note that the attached video crashes both Totem and VLC, so I don't feel bad about not playing it in firefox :)

Anyway, the patch is here if we want to support this kind busted video format (I didn't mention it but I verified that it works), I'll let the right people decide whether we want to land it (if Jeff gives the r+).
Anthony, what do you think?
Flags: needinfo?(ajones)
(In reply to Jean-Yves Avenard [:jya] from comment #15)
> Anthony, what do you think?

We should either:

a) play the busted video (given that Chrome plays it); OR
b) fail and show a useful error message
Flags: needinfo?(ajones)
Attachment #8704229 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 17

2 years ago
The consensus isn't clear, so I propose that I land the gfx fix, and if the decision is made to explicitly not support this kind of format, then we write a patch that cancels decoding somewhere closer to the decoding code itself. This way the decision will appear more intentional (we won't have to ask ourselves again every once in a while when we forget about this bug) and we'll have clearer contracts about what kind of input we can receive down the rendering pipeline (currently it cancels halfway in the middle so we can't expect much).

How does that sound Jean-Yves ?
Flags: needinfo?(jyavenard)
sounds good.
Flags: needinfo?(jyavenard)

Comment 19

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/aeceee13f667

Comment 20

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/aeceee13f667
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Nicolas, are you planning to request the uplift to aurora for this bug? merci
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 22

2 years ago
Comment on attachment 8704229 [details] [diff] [review]
allow cbcr planes that are bigger than the y plane (no matter how silly that is)

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: meh, broken video don't play anymore, with this patch we can play it, no clear consensus about if we are doing the web a service by being to play broken stuff, we just agreed that the place where it fails currently is not the proper one.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: none, reverts a small change back to how it was before.
[String/UUID change made/needed]:

uplifting this has no risk but is not very important either. FWIW, I'd uplift it just so that the behavior to stays consistent until we make the decision to support or to not support this kind of malformed video stream.
Flags: needinfo?(nical.bugzilla)
Attachment #8704229 - Flags: approval-mozilla-aurora?
Comment on attachment 8704229 [details] [diff] [review]
allow cbcr planes that are bigger than the y plane (no matter how silly that is)

Let's see how it goes!
Attachment #8704229 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox44: affected → wontfix
does not apply to aurora cleanly:

grafting 322992:aeceee13f667 "Bug 1235796 - Allow allocating YCbCr textures with Cb/Cr planes bigger than the Y plane. r=jrmuizel"
merging gfx/layers/ImageDataSerializer.cpp
warning: conflicts while merging gfx/layers/ImageDataSerializer.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 25

2 years ago
Ah, turns out I was wrong and the patch that caused the bug is not in aurora so there's no uplift to do.
status-firefox43: wontfix → unaffected
status-firefox44: wontfix → unaffected
status-firefox45: affected → unaffected
Flags: needinfo?(nical.bugzilla)
Attachment #8704229 - Flags: approval-mozilla-aurora+
(Reporter)

Comment 26

2 years ago
(In reply to Nicolas Silva [:nical] from comment #14)
> (In reply to Jean-Yves Avenard [:jya] from comment #13)
> > Personally here, I wouldn't apply those changes: the video is invalid, that
> > we used to play it was a bug :)
> > It's the media that needs fixing.
> [...]
> Note that the attached video crashes both Totem and VLC, so I don't feel bad
> about not playing it in firefox :)

Fixed in next release of VLC (v2.2.2): https://trac.videolan.org/vlc/ticket/16485

I would like to submit a bug to ffmpeg too, so that no more video will have the same defect. I found this bug: https://trac.ffmpeg.org/ticket/5049 Is it the same bug, or should I open a new ticket ? Also, not sure what I should explain in the bug report. I have the source mpeg file, the output .webm file and the command used to encode, but what is the exact problem ?
(Reporter)

Comment 27

2 years ago
I opened a bug report on ffmpeg, and they ask what is wrong with the .webm file. If someone knows, please answer either here (and I will send them your answer), or directly to the ffmpeg bug report: https://trac.ffmpeg.org/ticket/5214
Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(jyavenard)
the problem is described in comment 6.
The sizes of the YCbCr planes are nonsensical.

now it could very well be a libvpx issue with nothing to do with ffmpeg seeing that the bug was reported against libvpx
Flags: needinfo?(jyavenard)
(Reporter)

Comment 29

2 years ago
Thanks a lot, I reported the bug to the libvpx library: https://bugs.chromium.org/p/webm/issues/detail?id=1142
I don't see what's there to report, as to me this is more a client issue.
(Assignee)

Updated

2 years ago
Flags: needinfo?(nical.bugzilla)
(Reporter)

Comment 31

2 years ago
The libvpx asked the following question, which I don't know how to answer:
"what are the sizes you are seeing, the file header and decoded output says 320x240 YUV444."

I would need someone to explain where the problem in the video file is.
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.