Closed Bug 1457499 Opened 2 years ago Closed 2 years ago

Green band on some video

Categories

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

61 Branch
x86_64
Windows 10
defect

Tracking

()

VERIFIED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 + verified
firefox62 --- verified

People

(Reporter: zeusex81, Assigned: jya)

References

()

Details

(Keywords: nightly-community, regression)

Attachments

(6 files, 1 obsolete file)

Since a few days when playing some videos I have a green band on top.
For example : https://www.dailymotion.com/video/x6ign45
With 720p quality only gives me gives me *see attachment.
Component: General → Audio/Video: Playback
Product: Firefox → Core
Updated your video drivers recently?

Can you provide a copy of about:support
Flags: needinfo?(zeusex81)
Attached file about support.txt
Oh yes it seems to be related to the hardware decoding I disabled, the bug disappear when I enable it back.
Still there is an issue with software decoding.
Flags: needinfo?(zeusex81)
Component: Audio/Video: Playback → Graphics: WebRender
I could reproduce the problem with "media.hardware-video-decoding.enabled: false".
Assignee: nobody → sotaro.ikeda.g
Win10 1803, Nvidia GeForce GTX 1060

mozregression --launch 2018-05-10 --pref gfx.webrender.enabled:false media.hardware-video-decoding.enabled:false startup.homepage_welcome_url:'https://www.dailymotion.com/video/x6ign45'
mozregression --launch 2018-05-10 --pref layers.acceleration.disabled:true media.hardware-video-decoding.enabled:false startup.homepage_welcome_url:'https://www.dailymotion.com/video/x6ign45'

Reproducible with WebRender, Direct3D 11 (Advanced Layers) and Basic. This is not a WebRender bug.
Status: UNCONFIRMED → NEW
Has STR: --- → yes
Ever confirmed: true
Per comment 4
Component: Graphics: WebRender → Audio/Video: Playback
Keywords: regression
Summary: Regression : Green band on some video → Green band on some video
It seems that the problem happened at WMFVideoMFTManager::CreateBasicVideoFrame(). It expected that decoded image side is WMFVideoMFTManager::mImageSize. But actual decode size could be a bit bigger than the mImageSize.
With attachment 8975735 [details] [diff] [review], the problem was addressed for me.
Comment on attachment 8975735 [details] [diff] [review]
patch - Get decoded video size from IMFMediaType

Review of attachment 8975735 [details] [diff] [review]:
-----------------------------------------------------------------

IIRC, we had issue with VP9 images from the decoder when doing so.

The decoded picture size is described in the SPS NAL and we should be using that.
Thanks for the advice. By Comment 10, it is better to be handled by media team.
Assignee: sotaro.ikeda.g → nobody
Thanks for the help Sotaro.. I'll have a look at it later.
We did used to retrieve the image size that way earlier on... don't recall precisely why that was changed, but I remember there were issues with aspect ratio too.
Flags: needinfo?(jyavenard)
I have a patch ready, just having issues uploading to reviewboard following bootstrap updating mercurial, and cinnabar no longer working...
Flags: needinfo?(jyavenard)
Great news, thanks for the update!
Assignee: nobody → jyavenard
Priority: -- → P1
Comment on attachment 8976215 [details]
Bug 1457499 - P1. Remove unused variable.

https://reviewboard.mozilla.org/r/244406/#review250502
Attachment #8976215 - Flags: review?(bvandyk) → review+
Comment on attachment 8976216 [details]
Bug 1457499 - P2. Fix compilation warning.

https://reviewboard.mozilla.org/r/244408/#review250504
Attachment #8976216 - Flags: review?(bvandyk) → review+
Comment on attachment 8976217 [details]
Bug 1457499 - P3. Query decoded frame size instead of calculating it.

https://reviewboard.mozilla.org/r/244410/#review250510
Attachment #8976217 - Flags: review?(bvandyk) → review+
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f6b1c3486b39
P1. Remove unused variable. r=bryce
https://hg.mozilla.org/integration/autoland/rev/49047259d828
P2. Fix compilation warning. r=bryce
https://hg.mozilla.org/integration/autoland/rev/f2b4d5d46f68
P3. Query decoded frame size instead of calculating it. r=bryce
Please request Beta approval on this when you get a chance.
Flags: needinfo?(jyavenard)
Comment on attachment 8976215 [details]
Bug 1457499 - P1. Remove unused variable.

Approval Request Comment
[Feature/Bug causing the regression]: not a regression. However it never appeared before, likely due to the change in h264 encoding used by some sites.
[User impact if declined]: Popular web site video will show corrupted pictures on some system
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no (only manually)
[Needs manual test from QE? If yes, steps to reproduce]: yes. The problem will only be seen on Windows platform with hardware decoding disabled (such as running VMWare, system using the basic compositor etc)
[List of other uplifts needed for the feature/fix]: all three patches
[Is the change risky?]: unlikely
[Why is the change risky/not risky?]: the fix does rely on an expected (and undocumented) behaviour, but we have put diagnostic asserts to verify those assumptions, so crashes will quickly show on beta if the fix isn't valid.
[String changes made/needed]: none
Flags: needinfo?(jyavenard)
Attachment #8976215 - Flags: approval-mozilla-beta?
(In reply to Jean-Yves Avenard [:jya] from comment #24)
> [Why is the change risky/not risky?]: the fix does rely on an expected (and
> undocumented) behaviour, but we have put diagnostic asserts to verify those
> assumptions, so crashes will quickly show on beta if the fix isn't valid.
> [String changes made/needed]: none

Just FYI, diagnostic asserts are only enabled for DevEdition builds on Beta. Probably noticeable enough still, but FYI.
Comment on attachment 8976215 [details]
Bug 1457499 - P1. Remove unused variable.

Approved for 61.0b7, we'll see if anything interesting turns up over the next couple weeks on Beta.
Attachment #8976215 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Verified fixed on newest Nightly with the following specs. Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0

Will test with 61.0b7 tomorrow.
Verified fixed on 61.0b7 with the following specs: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: qe-verify+
Duplicate of this bug: 1463385
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.