1131638 - Multiple AMD video cards have issues displaying 1080@60fps videos

Status: VERIFIED FIXED

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

Description

[Bogdan Maris, Desktop QA]

Bug logged based on bug 1120128 comment 42.

In bug 1120128 AMD Radeon HD5800 video card was blacklisted so that 1080p60fps videos would work smooth. I was able to still see the issue on two more AMD cards, HD6450 and HD7700 series.

STR:
1. Visit https://www.youtube.com/watch?v=7SRTEXSpcyI
2. Change the quality of the video to 1080p60fps

Expected: Video runs smooth.

Actual: Video has a lot of black frames and some stuttering.

Graphics
Adapter Description	AMD Radeon HD 6450
Adapter Drivers	aticfx64 aticfx64 aticfx64 aticfx32 aticfx32 aticfx32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva atiumd6a atitmm64
Adapter RAM	1024
Device ID	0x6779
Direct2D Enabled	true
DirectWrite Enabled	true (6.3.9600.17111)
Driver Date	11-20-2014
Driver Version	14.501.1003.0
GPU #2 Active	false
GPU Accelerated Windows	1/1 Direct3D 11 (OMTC)
Subsys ID	21251462
Vendor ID	0x1002
WebGL Renderer	Google Inc. -- ANGLE (AMD Radeon HD 6450 Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote	true
AzureCanvasBackend	direct2d 1.1
AzureContentBackend	direct2d 1.1
AzureFallbackCanvasBackend	cairo
AzureSkiaAccelerated	0

Graphics
Adapter Description AMD Radeon HD 7700 Series
Adapter Drivers aticfx64 aticfx64 aticfx64 aticfx32 aticfx32 aticfx32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva atiumd6a atitmm64
Adapter RAM 1024
Device ID 0x683f
Direct2D Enabled true
DirectWrite Enabled true (6.3.9600.17111)
Driver Date 11-20-2014
Driver Version 14.501.1003.0
GPU #2 Active false
GPU Accelerated Windows 1/1 Direct3D 11 (OMTC)
Subsys ID 00000000
Vendor ID 0x1002
WebGL Renderer Google Inc. -- ANGLE (AMD Radeon HD 7700 Series Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote true
AzureCanvasBackend direct2d 1.1
AzureContentBackend direct2d 1.1
AzureFallbackCanvasBackend cairo
AzureSkiaAccelerated 0

Catalyst Version: 14.12 AMD Catalyst Omega Software

Comment 1

[Bobby Holley (:bholley)]

Do we need to expand the blacklist, or does this indicate that it's a pan-GPU driver issue?

Comment 2

[Matt Woodrow (:mattwoodrow)]

I suspect we need to blacklist some more cards, but I'm looking into a black flashing issue with Anthony's machine that could be related.

Comment 3

[Toshihiro Yamada]

On Win7.1 with Radeon HD7770, the video plays normally.

Comment 4

[Matt Woodrow (:mattwoodrow)]

Can you please test with the builds below to see if this still happens.

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-6db0d6b003f5/try-win32/

Comment 5

[John Doe]

Windows 8.1 64bit, Radeon HD5770.
The video "lags" but no black frames with the try build.

Comment 6

[Bogdan Maris, Desktop QA]

(In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> Can you please test with the builds below to see if this still happens.
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mwoodrow@mozilla.
> com-6db0d6b003f5/try-win32/

AMD Radeon HD6450/Windows 8.1 64bit 
- No black frames, but as annaeus said in comment 5 if I switch to full screen the video lags.

AMD Radeon HD7700/Windows 8.1 64bit
- No black frames, no lags, works just fine.

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Discard frames that fail to sync

This fixes the black frames by just dropping them instead.

We might want to consider recording how often this happens, and falling back to software decoding if it's too frequent (though software might still be worse, so it's a bit complex).

Comment 8

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8568262 [details] [diff] [review]
Discard frames that fail to sync

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

::: gfx/layers/D3D9SurfaceImage.cpp
@@ +167,5 @@
> +      mIsValid = false;
> +    }
> +    break;
> +  }
> +  

Fixed whitespace locally.

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8568262 [details] [diff] [review]
Discard frames that fail to sync

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

::: gfx/layers/D3D9SurfaceImage.cpp
@@ +157,2 @@
>  
> +  int iterations = 0;

Should probably comment here why we do this. Also the comment on line 139 implies that we won't be waiting, but that's not the case now that we sleep here.

Comment 10

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/78b34d0e414a

Comment 11

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1b55b99dc432

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/78b34d0e414a
https://hg.mozilla.org/mozilla-central/rev/1b55b99dc432

Comment 13

[Hazel Ward]

I am still experiencing this issue with the latest nightly. 720@60 works flawlessly, 1080 barely works at all. Sapphire Tri-X OC R9 290X with latest drivers

Comment 14

[Gustavo Silva]

It seems this patch has fixed the flashing black frames, but 1080p@60fps videos still barely play for me, with an extremely low frame rate. My graphics info from about:support:

Adapter Description: AMD Radeon HD 6800 Series
Adapter Drivers: aticfx64 aticfx64 aticfx64 aticfx32 aticfx32 aticfx32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva atiumd6a atitmm64
Adapter RAM: 1024
Device ID: 0x6738
Direct2D Enabled: true
DirectWrite Enabled: true (6.3.9600.17415)
Driver Date: 11-20-2014
Driver Version: 14.501.1003.0
GPU #2 Active: false
GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
Subsys ID: 174b174b
Vendor ID: 0x1002
WebGL Renderer: Google Inc. -- ANGLE (AMD Radeon HD 6800 Series Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote: true
AzureCanvasBackend: direct2d 1.1
AzureContentBackend: direct2d 1.1
AzureFallbackCanvasBackend: cairo
AzureSkiaAccelerated: 0

Comment 15

[Hazel Ward]

Same issue for me, actually. Here's my about:support information

Adapter Description	AMD Radeon R9 200 Series
Adapter Drivers	aticfx64 aticfx64 aticfx64 aticfx32 aticfx32 aticfx32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva atiumd6a atitmm64
Adapter RAM	4095
Device ID	0x67b0
Direct2D Enabled	true
DirectWrite Enabled	true (6.3.9600.17415)
Driver Date	11-20-2014
Driver Version	14.501.1003.0
GPU #2 Active	false
GPU Accelerated Windows	1/1 Direct3D 11 (OMTC)
Subsys ID	00000000
Vendor ID	0x1002
WebGL Renderer	Google Inc. -- ANGLE (AMD Radeon R9 200 Series Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote	true
AzureCanvasBackend	direct2d 1.1
AzureContentBackend	direct2d 1.1
AzureFallbackCanvasBackend	cairo
AzureSkiaAccelerated	0

Comment 16

[Matt Woodrow (:mattwoodrow)]

I have a machine with a radeon card now, am investigating this further.

Comment 17

[Matt Woodrow (:mattwoodrow)]

Backing this out for causing a bunch of regressions.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3bd19a1cf4b4

Comment 18

[Matt Woodrow (:mattwoodrow)]

Attachment: Discard frames that fail to sync v2

Comment 19

[Matt Woodrow (:mattwoodrow)]

Attachment: Record invalid frames as dropped for video playback stats

Comment 20

[Matt Woodrow (:mattwoodrow)]

Attachment: Disable hardware decoding if too many frames are invalid

Comment 21

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8575734 [details] [diff] [review]
Disable hardware decoding if too many frames are invalid

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

Looks fine, but if we can use MP4Reader::DecoderData::mError instead of adding a very similar MP4Reader::DecoderData::mPendingError, we should.

::: dom/media/MediaDecoderStateMachine.cpp
@@ +2909,5 @@
>    if (container) {
>      if (aData->mImage && !aData->mImage->IsValid()) {
>        MediaDecoder::FrameStatistics& frameStats = mDecoder->GetFrameStatistics();
>        frameStats.NotifyCorruptFrame();
> +      if (frameStats.GetCorruptedFrames() == MAX_CORRUPTED_FRAMES) {

Maybe we should only attempt to disable hardware acceleration if mReader->VideoIsHardwareAccelerated() returns true?

@@ +2910,5 @@
>      if (aData->mImage && !aData->mImage->IsValid()) {
>        MediaDecoder::FrameStatistics& frameStats = mDecoder->GetFrameStatistics();
>        frameStats.NotifyCorruptFrame();
> +      if (frameStats.GetCorruptedFrames() == MAX_CORRUPTED_FRAMES) {
> +        DecodeTaskQueue()->Dispatch(new DisableHardwareAccelerationRunnable(mReader));

You should be able to use:

NS_NewRunnableMethod(mReader, &MediaDecoderReader::DisableHardwareAcceleration)

Right?

::: dom/media/fmp4/MP4Reader.cpp
@@ +594,5 @@
> +      mVideo.mError = true;
> +      if (mVideo.HasPromise()) {
> +        mVideo.RejectPromise(DECODE_ERROR, __func__);
> +      } else {
> +        mVideo.mPendingError = true;

Can we get away with using mError here instead?

@@ +644,5 @@
>    }
>  
>    MonitorAutoLock lock(mVideo.mMonitor);
>    nsRefPtr<VideoDataPromise> p = mVideo.mPromise.Ensure(__func__);
> +  if (mVideo.mPendingError) {

And can we use mError here too?

If we've previously error'd we should be unable to continue decoding.

::: dom/media/mediasource/MediaSourceReader.h
@@ +84,5 @@
>  
>    void NotifyTimeRangesChanged();
>  
> +  virtual void DisableHardwareAcceleration() MOZ_OVERRIDE {
> +    if (GetVideoReader()) {

Doesn't this need to take the decoder monitor in order to be threadsafe?

Comment 22

[Matt Woodrow (:mattwoodrow)]

Attachment: Disable hardware decoding if too many frames are invalid v2

Comment 23

[Matt Woodrow (:mattwoodrow)]

> > +  virtual void DisableHardwareAcceleration() MOZ_OVERRIDE {
> > +    if (GetVideoReader()) {
> 
> Doesn't this need to take the decoder monitor in order to be threadsafe?

Do we? Other accesses to GetVideoReader() don't hold the monitor.

Comment 24

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/be68ba0145b1
https://hg.mozilla.org/integration/mozilla-inbound/rev/02418459ed82
https://hg.mozilla.org/integration/mozilla-inbound/rev/115abd2ff483

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/be68ba0145b1
https://hg.mozilla.org/mozilla-central/rev/02418459ed82
https://hg.mozilla.org/mozilla-central/rev/115abd2ff483

Comment 26

[Lawrence Mandel [:lmandel] (use needinfo)]

Matt - 37 is marked as affected. wdyt about uplifting to Beta?

Comment 27

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8575731 [details] [diff] [review]
Discard frames that fail to sync v2

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Black video frames with a lot of AMD video cards
[Describe test coverage new/current, TreeHerder]: Tested manually, few days on nightly.
[Risks and why]: Fairly low risk, very important to MSE playback performance.
[String/UUID change made/needed]: None

Comment 28

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8575731 [details] [diff] [review]
Discard frames that fail to sync v2

MSE is set to release in 37. Let's take this fix in Beta 6 now that it's had a few days of testing on Nightly. Beta+

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dd6e425826fc
https://hg.mozilla.org/releases/mozilla-aurora/rev/fa9401887405
https://hg.mozilla.org/releases/mozilla-aurora/rev/2a76dd8f86c2

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6d4536c5fa38
https://hg.mozilla.org/releases/mozilla-beta/rev/772774b9be23
https://hg.mozilla.org/releases/mozilla-beta/rev/d35ca21dada8

Comment 31

[Ryan VanderMeulen [:RyanVM]]

Backed out for bustage. Looks like it depends on bug 1128380.
https://hg.mozilla.org/releases/mozilla-aurora/rev/4005301169f1
https://hg.mozilla.org/releases/mozilla-beta/rev/3a27c2da51d1

https://treeherder.mozilla.org/logviewer.html#?job_id=678988&repo=mozilla-aurora

Comment 32

[Ryan VanderMeulen [:RyanVM]]

Windows beta builds are still broken even after including bug 1128380 due to bug 1097699 only being on 38+.
https://treeherder.mozilla.org/logviewer.html#?job_id=5681916&repo=try

Comment 33

[Matt Woodrow (:mattwoodrow)]

Attachment: Rebased part 1 for beta

Comment 35

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d7647fcac087
https://hg.mozilla.org/releases/mozilla-aurora/rev/8b455f330594
https://hg.mozilla.org/releases/mozilla-aurora/rev/ce270a92ad19

https://hg.mozilla.org/releases/mozilla-beta/rev/297e2e626fe9
https://hg.mozilla.org/releases/mozilla-beta/rev/f88fcb8ccc27
https://hg.mozilla.org/releases/mozilla-beta/rev/edb24ca59d13

Comment 37

[Bogdan Maris, Desktop QA]

I verified that the issue does not reproduce anymore on Firefox 37.0 RC build 2 and Firefox 38 beta 1. I will go ahead and close this issue for now.
Robert and Gustavo, can you still reproduce on this builds?
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/37.0-candidates/build2/win32/en-US/Firefox%20Setup%2037.0.exe
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/38.0b1-candidates/build1/win32/en-US/Firefox%20Setup%2038.0b1.exe

Comment 38

[Gustavo Silva]

Sorry for the delay in responding. The issue no longer occurs on current builds.

Comment 39

[Hazel Ward]

Sorry for the delay as well, WFM