Closed Bug 1236112 Opened 9 years ago Closed 9 years ago

Full screen 60fps videos cause constant screen shake

Categories

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

Product:
Core
Component:
Audio/Video: Playback
Version:
43 Branch
Platform:
x86
Windows 10
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla48
Tracking Flags:
Tracking Status
firefox44 --- wontfix
firefox45 + wontfix
firefox46 + wontfix
firefox47 + fixed
firefox48 --- fixed

People

(Reporter: Steve.hack, Assigned: mattwoodrow, NeedInfo)

References

Details

Attachments

(1 file)

flush-d3d9-video-sync
8.15 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0 Build ID: 20151208100201 Steps to reproduce: Go fullscreen in any 60fps video on Youtube. Actual results: Constant screen shake. See: https://youtu.be/2ejiiSTHwV8 Expected results: Smooth video playback. In comparison Microsoft Edge works flawlessly.
OS: Unspecified → Windows 10
Hardware: Unspecified → x86
Sent from my PC but the device shown in the video is a Lenovo Yoga 2 10" Windows 10 tablet.
I should also mention this does not happen on my gaming PC, but that has an i5 3570k @ 4.3GHz / GTX 970 so it merely brute forces the issue, the tablet isn't so lucky but as I said, it can run 1080p/60fps without issue in Edge, just not Firefox nor Chrome.
Summary: Fullscreen on Youtube causes constant screen shake at 60fps → Full screen 60fps videos cause constant screen shake
It's not just Youtube, it's any site that offers HTML5 60fps videos, Gamersyde.com for example. Once again Edge runs them perfectly.
I don't understand the differences with them.
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
See Also: → 1236107
(In reply to YF (Yang) from comment #4) > I don't understand the differences with them. The screen shake only happens in full screen (or more accurately whenever I make the video player larger than default) wheras the high CPU usage happens no matter the window size. Basically I'm having lots of issues with HTML5 and 60fps content unless I use Edge. Also I downloaded a very high quality trailer at 1080p/60fps and it ran perfectly offline in media player classic so the tablet has the spec to run it, but not inFirefox for some reason.
Priority: -- → P1
Can you please paste in the graphics section of about:support?
[Tracking Requested - why for this release]: We appear to be displaying frames out of order under some circumstances on Windows. This needs further investigation
tracking-firefox45: --- → ?
Tracking as it could be an important issue. Jean-Yves, does it ring a bell? Thanks
status-firefox45: --- → affected
tracking-firefox45: ? → +
Flags: needinfo?(jyavenard)
Unfortunately, no.. This appears to be a windows only issue. We believe to be a race in the gfx code.
Flags: needinfo?(jyavenard)
Do you think it is actionable in the 45 timeframe? Thanks
Hard to say.. It will depend if we can reproduce it or not. The reported hasn't responded to our request for information. We do not know what graphic cards his uses or much details about his configuration
(In reply to Jean-Yves Avenard [:jya] from comment #12) > Hard to say.. > > It will depend if we can reproduce it or not. > The reported hasn't responded to our request for information. We do not know > what graphic cards his uses or much details about his configuration The reporter listed their two machines, a Yoga 2 and a desktop with a GTX 970. I no longer own it but I previously owned a Yoga 2 which had onboard Intel graphics. I have this problem with my current desktop (Windows 10 x64, GTX 980ti) and my previous desktop configuration (Windows 10 x64, GTX 970 - separate install, not upgrade). See bug #1233815 if you need more on my configuration but it's basically the same problem - albeit much, much harder to reproduce for me than it seems to be for the reporter of this bug. FWIW, for me it applies to all html5 videos, fullscreen or not. It's easier to notice in fullscreen though and isn't 100% reproducible for me. It comes and goes.
you mentioned that it was something new. Any chance you could use the mozregression tool to find out which commit introduced the problem? it shouldn't take too long hopefully. So long as you now approximately when a problem started. http://mozilla.github.io/mozregression/install.html
It takes days for me to reproduce, so mozregression isn't going to be any use. It seems like for Steve.hack it's very easy to reproduce, though, so maybe they could use mozregression on it?
https://www.youtube.com/watch?v=xvOAZ0TWWgg reproduces this for me (for the record, having a link to an actual video instead of saying "any video that..." would have saved me a bit of time hunting around in the beginning). I can also reproduce this getting worse between Fx42 and Fx43.
Note that the controls need to be visible for it to reproduce AFAICT.
Moving the mouse around makes it easier to hit too. Mozregression is pointing at bug 1217185 for me. So I guess now I need to try to re-bisect this with e10s disabled to find what broke this on non-e10s builds.
Blocks: 1217185
No longer blocks: 1243619
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(Steve.hack)
See Also: → 1243619
With e10s disabled, I get the following regression range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c48f8f05e6d9eea2af3f20ac8c49eb8aa653d0f8&tochange=e26240900b5aa9432c69bcbe39f64dafac60be86 I'm guessing this is from bug 1198202? At least the regression range fits this breaking in 43.
status-firefox44: --- → wontfix
status-firefox46: --- → affected
status-firefox47: --- → affected
tracking-firefox46: --- → ?
tracking-firefox47: --- → ?
Flags: needinfo?(matt.woodrow)
On Mac; with 44; sometimes having the playback cursor displayed; hovering over it with the mouse causes flickering but only if the overlay. Not the whole image. Not sure if this is related.
Both of those regression ranges point to when DXVA was enabled for that specific configuration. I strongly suspect that we're racing on access the D3D9 textures. The H264 MF Transform maintains a pool of D3D9 textures to decode into, and I suspect that it's reusing one before we've managed to copy the pixels we need out of it. Unfortunately we don't have the source code for the MF Transform (it's provided as part of Windows), but this MSDN article [1] describes how to write our own one and I assume matches the behaviour fairly closely. Parts that stands out is "Call IMFTrackedSample::SetAllocator and provide a pointer to the IMFAsyncCallback interface, implemented by the decoder. When the video renderer releases the sample, the decoder's callback will be invoked." and "Use the callback from the SetAllocator method (step 3) to keep track of which samples are currently available and which are in use." My understanding of this, is that when the last reference to the IMFSample is released by our code, a callback fires within the MF Transform that marks the surface as being available for use again. Our code for copying data out of the IMFSamples is asynchronous, but we don't hold on to a pointer to the IMFSample while we wait, only the D3D9 texture. It's believable that these things could be racy, since our copy/colour conversion uses the standard GPU paths, and decoding uses specialized silicon. I doubt D3D9 provides synchronization between these. A simple solution would be to hold a reference to the IMFSample in the D3D9SurfaceImage, and release it when the query completes. I'm not sure what happens if the decoder thread runs out of available samples though, especially if for some reason the MDSM isn't flushing the existing ones. Little worried out deadlock in that case. A better fix might be to keep the output within the decoder thread, and only pass them back through the Output() callback once we've finished resolving them and we know the IMFSample can be released. That's a bigger refactoring though. It's also possible that bug 1079533 could fix this, since the colour conversion should instead happen on dedicated silicon, and this might enforce ordering with the decoder. Experimenting with the MF_SA_MINIMUM_OUTPUT_SAMPLE_COUNT attribute might make this problem easier/harder to reproduce. I don't think that's a good long term fix. I won't have time to look into this more over the next few weeks, Chris or Jean-Yves, do either of you have time to play with with? [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa965266%28v=vs.85%29.aspx
Flags: needinfo?(matt.woodrow) → needinfo?(cpearce)
Joe, do you know if VideoProcessBlt would have defined ordering with DXVA video decoding, or would we still need to synchronize ourselves? I realize that the answer to that might be driver/hardware/architecture specific. Thanks!
Flags: needinfo?(joseph.k.olivas)
I remember when we had that issue on Mac, all we had to do is set a pointer somewhere and the IOsurface wouldn't get recycled until it had been composited... Isn't the same done with D3D surface?? Why would it be any different on Windows than on Mac? The Mac decoder too uses its own pool of frames.
Could someone please test with the pref 'media.windows-media-foundation.allow-d3d11-dxva' set to true (on Windows 8 or higher) and see if that helps at all?
Flags: needinfo?(ryanvm)
Works flawlessly. No dropped frames either (and video is noticeably smoother!).
Flags: needinfo?(ryanvm)
Awesome, thanks Ryan! The D3D11 DXVA version of the code uses the 'CLSID_VideoProcessorMFT' MFTransform for doing colour conversion, which I suspect is similar to VideoProcessBlt (and possibly uses that internally). I checked what Chromium is doing, and they indeed hold a reference to the IMFSample for the duration of the colour conversion copy, but only when using d3d9. I think it's worth trying to get the VideoProcessBlt code working for d3d9 (since it should be faster, and fixes our colour-space problem), and it may very well fix this bug for Windows 7 users. Bug 1079533 was filed for the potential perf wins of VideoProcessBlt, and bug 1161349 is for the colour-space problems. The latter has a patch, but unfortunately it works everywhere except NVIDIA cards. Was anyone able to reproduce this bug on an intel or AMD GPU? I've opened bug 1248496 to get D3D11 DXVA enabled by default, that'll make life better for Windows 8+ users.
Steve, what GPU do you have in the Yoga 2? Can you please paste in the graphics section of about:support? Thanks!
Flags: needinfo?(Steve.hack)
To clarify, VideoProcessBlt is part of the d3d9 interface to DXVA, the D3D11 MFTransform almost certainly uses the d3d11 equivalent: ID3D11VideoContext/ID3D11VideoProcessor. I think it's reasonable to believe that the d3d9 one would fix this bug in the same way though :)
(In reply to Matt Woodrow (:mattwoodrow) from comment #24) > Could someone please test with the pref > 'media.windows-media-foundation.allow-d3d11-dxva' set to true (on Windows 8 > or higher) and see if that helps at all? Which builds have this pref? I don't see it with a default value of false (or otherwise) in Developer Edition. I'll set it anyway and see what happens, but I'm guessing maybe I need Nightly?
(In reply to K. Gadd (:kael) from comment #29) > > Which builds have this pref? I don't see it with a default value of false > (or otherwise) in Developer Edition. I'll set it anyway and see what > happens, but I'm guessing maybe I need Nightly? All of them up to release. It's just not in about:config by default, adding it yourself should work.
I was able to reproduce this on Intel HW on this video: https://www.youtube.com/watch?v=79ImZE0K7xc After going into fullscreen, at about 10 seconds in (when the guy in the blue helmet gets near the camera), the video gets shaky and goes to hell :) Setting media.windows-media-foundation.allow-d3d11-dxva to true seems to fix this. As for the enforced ordering for VideoProcessBlt, I need to check. I'll leave needinfo open. Here's my adapter info. Adapter Description Intel(R) HD Graphics 4600 Adapter Drivers igdumdim64 igd10iumd64 igd10iumd64 igd12umd64 igdumdim32 igd10iumd32 igd10iumd32 igd12umd32 Adapter RAM Unknown Asynchronous Pan/Zoom none Device ID 0x0416 Direct2D Enabled true DirectWrite Enabled true (10.0.10586.0) Driver Date 11-18-2015 Driver Version 20.19.15.4331 GPU #2 Active false GPU Accelerated Windows 1/1 Direct3D 11 (OMTC) Subsys ID 22128086 Supports Hardware H264 Decoding Yes Vendor ID 0x8086 WebGL Renderer Google Inc. -- ANGLE (Intel(R) HD Graphics 4600 Direct3D11 vs_5_0 ps_5_0) windowLayerManagerRemote true AzureCanvasBackend direct2d 1.1 AzureContentBackend direct2d 1.1 AzureFallbackCanvasBackend cairo AzureSkiaAccelerated 0
Answer I got on VideoProcessBlt is that it's a MSFT question :) Sorry I couldn't provide more on it.
Flags: needinfo?(joseph.k.olivas)
(In reply to Joe Olivas from comment #32) > Answer I got on VideoProcessBlt is that it's a MSFT question :) Sorry I > couldn't provide more on it. No worries, it was worth a shot. Will just have to test it instead! I've done a build with VideoProcessBlt for d3d9: http://archive.mozilla.org/pub/firefox/try-builds/mwoodrow@mozilla.com-f5c90693d91e885f78fe9ec01d65642efda22f66/try-win32/ Could you (or anyone else that can reproduce this) test this please? It will need media.windows-media-foundation.allow-d3d11-dxva to be set to false again to be useful. Thanks! I wasn't able to reproduce on my HD 3000, and the patch still doesn't work for NVIDIA :(
Flags: needinfo?(joseph.k.olivas)
Ryan, any chance you could check this one too please? (With the d3d11 pref set to false) Thanks!
Flags: needinfo?(ryanvm)
(In reply to Matt Woodrow (:mattwoodrow) from comment #33) > Could you (or anyone else that can reproduce this) test this please? It will > need media.windows-media-foundation.allow-d3d11-dxva to be set to false > again to be useful. I tried the build this morning and the video plays about 5x slower, but no jerkiness :) I'll take a look at it again this afternoon and see if I can find anything obvious.
Flags: needinfo?(joseph.k.olivas)
Is there an updated/rebased patch for VideoProcessBlt that I can apply locally?
Flags: needinfo?(matt.woodrow)
(In reply to Joe Olivas from comment #36) > Is there an updated/rebased patch for VideoProcessBlt that I can apply > locally? https://hg.mozilla.org/try/raw-rev/686371be9d4a It's re-creating the video processor each frame, that could be related to the slowness.
Flags: needinfo?(matt.woodrow)
I'm seeing 50% of clocks in igdumdim32.dll stemming from CVideoProcessorDevice::VideoProcessBlt, and only 2.9% in cVideoAccelerationService::CreateVideoProcessor. 19% of our clocks are in xul.dll, but nothing significant; everything is under 0.5%. Hottest stack to the VideoProcessBlt makes sense with the new patch: igdumdim32.dll ! func@0x10138450 - [unknown source file] igdumdim32.dll ! func@0x10158920 + 0x19 - [unknown source file] igdumdim32.dll ! func@0x1025b870 + 0x82 - [unknown source file] igdumdim32.dll ! func@0x1032eb50 + 0x2d7 - [unknown source file] igdumdim32.dll ! func@0x10261a80 + 0xd4 - [unknown source file] igdumdim32.dll ! func@0x1025b48f + 0x1cc - [unknown source file] igdumdim32.dll ! func@0x101b69d0 + 0x14a - [unknown source file] igdumdim32.dll ! func@0x101b7250 + 0xd3 - [unknown source file] igdumdim32.dll ! func@0x101b7610 + 0xc6 - [unknown source file] igdumdim32.dll ! func@0x101b79f0 + 0x65f - [unknown source file] igdumdim32.dll ! func@0x101b8535 + 0x18 - [unknown source file] igdumdim32.dll ! func@0x101798ef + 0x1fb - [unknown source file] igdumdim32.dll ! func@0x1016222f + 0xbd - [unknown source file] d3d9.dll ! VideoProcessBltLH + 0x160 - [unknown source file] d3d9.dll ! CVideoProcessDevice::VideoProcessBlt + 0xf0 - [unknown source file] dxva2.dll ! CVideoProcessorDevice::VideoProcessBlt + 0x3b0 - [unknown source file] xul.dll ! mozilla::layers::D3D9SurfaceImage::AllocateAndCopy + 0x39f - d3d9surfaceimage.cpp:140 xul.dll ! mozilla::D3D9DXVA2Manager::CopyToImage + 0x88 - dxva2manager.cpp:427 xul.dll ! mozilla::WMFVideoMFTManager::CreateD3DVideoFrame + 0x6d - wmfvideomftmanager.cpp:536 xul.dll ! mozilla::WMFVideoMFTManager::Output + 0x139 - wmfvideomftmanager.cpp:622 xul.dll ! mozilla::WMFMediaDataDecoder::ProcessOutput + 0x33 - wmfmediadatadecoder.cpp:152 xul.dll ! mozilla::WMFMediaDataDecoder::ProcessDecode + 0x6b - wmfmediadatadecoder.cpp:144 xul.dll ! nsRunnableMethodImpl<void ( mozilla::dom::workers::ServiceWorkerJobBase::*)(enum nsresult),1,enum nsresult>::Run + 0x10 - nsthreadutils.h:870 xul.dll ! mozilla::TaskQueue::Runner::Run + 0x84 - taskqueue.cpp:170 xul.dll ! nsThreadPool::Run + 0x2f2 - nsthreadpool.cpp:227 xul.dll ! nsThread::ProcessNextEvent + 0x3f3 - nsthread.cpp:1018 xul.dll ! NS_ProcessNextEvent + 0x25 - nsthreadutils.cpp:297 xul.dll ! mozilla::ipc::MessagePumpForNonMainThreads::Run + 0xdd - messagepump.cpp:355 xul.dll ! MessageLoop::RunHandler + 0x52 - message_loop.cc:227 xul.dll ! MessageLoop::Run + 0x18 - message_loop.cc:201 xul.dll ! nsThread::ThreadFunc + 0xb2 - nsthread.cpp:411 nss3.dll ! PR_NativeRunThread + 0xc0 - pruthr.c:397 nss3.dll ! pr_root + 0xa - w95thred.c:95 ucrtbase.dll ! func@0x10086230 + 0x73 - [unknown source file] kernel32.dll ! BaseThreadInitThunk + 0x23 - [unknown source file] ntdll.dll ! _RtlUserThreadStart + 0x2e - [unknown source file] ntdll.dll ! _RtlUserThreadStart + 0x1a - [unknown source file]
Flags: needinfo?(ryanvm)
(In reply to Matt Woodrow (:mattwoodrow) from comment #24) > Could someone please test with the pref > 'media.windows-media-foundation.allow-d3d11-dxva' set to true (on Windows 8 > or higher) and see if that helps at all? FTR, I've had this set for a few days now and it seems to mitigate it for me. Is there a reason why I should prefer to try VideoProcessBlt instead if it's way slower?
Tracking for 46+. Matt, are you working on this one? It seems late to get in a fix for 45 as we're going to build for the 45 RC on Monday.
tracking-firefox46: ? → +
tracking-firefox47: ? → +
(In reply to K. Gadd (:kael) from comment #40) > (In reply to Matt Woodrow (:mattwoodrow) from comment #24) > > Could someone please test with the pref > > 'media.windows-media-foundation.allow-d3d11-dxva' set to true (on Windows 8 > > or higher) and see if that helps at all? > > FTR, I've had this set for a few days now and it seems to mitigate it for > me. Is there a reason why I should prefer to try VideoProcessBlt instead if > it's way slower? I seem to have spoken too soon. While the dxva pref seems to dramatically reduce the severity of the bug (I haven't ever seen *multiple* back-to-back frame glitches with it on, while I saw that very frequently before), the bug isn't gone. I saw at least 5 out-of-order frames during the first 5 minutes of watching https://www.youtube.com/watch?v=m8Sa5dUAXgw in fullscreen, and I'm seeing it intermittently in other html5 videos (gifs on twitter, etc). It's possible what I'm seeing is Bug 1206526 applied to video, though, and not the video processor problem, since 1206526 is still a near-constant menace for me.
jean-yves, any chance you can work on this or the related bug? Wontfix for 45 and 46; since no one is assigned it seems unlikely we will get a fix before 47.
status-firefox45: affected → wontfix
status-firefox46: affected → wontfix
Flags: needinfo?(jyavenard)
I'm afraid not.. This is a gfx / D3D bug, well outside my area of expertise.. Matt would be able to answer this question hopefully
Flags: needinfo?(jyavenard) → needinfo?(matt.woodrow)
So, according to the docs, we need to hold a ref to the IMFSample object for as long as we are using the texture. The decoder transform uses gets a callback when the IMFSample refcount drops to 1, and uses this to decide when to reuse the texture. We could store a reference to the IMFSample in the D3D9SurfaceImage, but we'd risk deadlocking the decoder if it ran out of samples, so we'd need to make sure we're frequently checking for the query completion and dropping the ref. Just synchronously blocking the decoder thread until the copy completes is a lot simpler and safer, but it's possible that it might reduce the concurrency between decoding and copying. That really depends on how the decoder MFT works. The other option is to change the code a bit so that the WMF Output function can return results asynchronously. That way we'd be able to post the query wait back to our own thread, and maybe queue up some more frames in the mean time. This is a lot more complicated change though. Chris, would you be able to test this on some of the low power devices that you used when writing this initially? I'll try spend a day in the Auckland office next week to help too.
Flags: needinfo?(matt.woodrow)
Attachment #8726922 - Flags: review?(cpearce)
Attachment #8726922 - Flags: review?(cpearce) → review+
Flags: needinfo?(cpearce)
(In reply to Matt Woodrow (:mattwoodrow) from comment #45) > Chris, would you be able to test this on some of the low power devices that > you used when writing this initially? I'll try spend a day in the Auckland > office next week to help too. I am unable to observe any performance difference with this patch applied on the low end devices that I have.
Matt - can we assign this bug to you?
Flags: needinfo?(matt.woodrow)
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #47) > Matt - can we assign this bug to you? Yes. (In reply to Chris Pearce (:cpearce) from comment #46) > I am unable to observe any performance difference with this patch applied on > the low end devices that I have. Awesome, thanks for testing this.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
(In reply to K. Gadd (:kael) from comment #42) > I seem to have spoken too soon. While the dxva pref seems to dramatically > reduce the severity of the bug (I haven't ever seen *multiple* back-to-back > frame glitches with it on, while I saw that very frequently before), the bug > isn't gone. I saw at least 5 out-of-order frames during the first 5 minutes > of watching https://www.youtube.com/watch?v=m8Sa5dUAXgw in fullscreen, and > I'm seeing it intermittently in other html5 videos (gifs on twitter, etc). > It's possible what I'm seeing is Bug 1206526 applied to video, though, and > not the video processor problem, since 1206526 is still a near-constant > menace for me. I think I understand why this is now. I've filed bug 1257013 to fix it.
Flags: needinfo?(matt.woodrow)
https://hg.mozilla.org/mozilla-central/rev/6d8001b49fea
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Blocks: 1257013
Comment on attachment 8726922 [details] [diff] [review] flush-d3d9-video-sync This patch is entirely superceeded by the patches in bug 1257013, and all the code that moves with this patch is deleted. It's just quite painful to rebase bug 1257013 around this, so we should just uplift this as well and make life easier.
Attachment #8726922 - Flags: approval-mozilla-aurora?
Comment on attachment 8726922 [details] [diff] [review] flush-d3d9-video-sync This is needed to uplift the patches from bug 1257013.
Attachment #8726922 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: