Closed Bug 1332952 Opened 6 years ago Closed 6 years ago

Paused WebM videos with alpha channel become completely transparent when hardware acceleration is disabled


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

Not set



Tracking Status
firefox53 --- verified
firefox54 --- verified


(Reporter: kkoorts, Assigned: kkoorts)




(1 file)

For videos opened in their own tab:
1. Pausing the video causes it to become transparent.
2. A looping video has an annoying transparent flash each time it passes the end of the video.

For videos embedded in a web page:
1. Pausing the video followed by clicking elsewhere to remove focus from the video causes it to become transparent.
2. Looping videos flash, same as in the own-tab case.
Assignee: nobody → kkoorts
Blocks: 944117
Comment on attachment 8829274 [details]
Bug 1332952 - Implement GetAsSourceSurface() for SharedRGBImage.

::: gfx/layers/ipc/SharedRGBImage.cpp:120
(Diff revision 1)
> +
> +  RefPtr<gfx::SourceSurface> surface;
> +  { //should this be in its own scope? copied structure from BasicPlanarYCbCrImage::GetAsSourceSurface()
> +    BufferTextureData* decoded_buffer =
> +      mTextureClient->GetInternalData()->AsBufferTextureData();
> +    RefPtr<gfx::DrawTarget> drawTarget = decoded_buffer->BorrowDrawTarget();

Please add a comment about how we're 'borrowing' the DT and retaining a permanent reference to the underlying data (via the surface), but that it's in this instance since we know that the TextureClient is always wrapping a BufferTextureData so it won't go away underneath us.
Attachment #8829274 - Flags: review?(matt.woodrow) → review+
Keywords: checkin-needed
Pushed by
Implement GetAsSourceSurface() for SharedRGBImage. r=mattwoodrow
Keywords: checkin-needed
Comment on attachment 8829274 [details]
Bug 1332952 - Implement GetAsSourceSurface() for SharedRGBImage.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 944117 - WebM alpha support
[User impact if declined]: WebM alpha will not work correctly (frames go completely transparent when paused) when hardware acceleration is disabled.
[Is this code covered by automated tests?]: There is a reftest for the general case of alpha working in WebM videos, but not for this specific case as the test machines have hardware acceleration enabled.
[Has the fix been verified in Nightly?]: I have verified it
[Needs manual test from QE? If yes, steps to reproduce]: No it's not a commonly used feature.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: It is not risky as it implements a method that doesn't affect anything besides WebM videos with alpha, and is straightforward to implement.
[String changes made/needed]: No string changes needed
Attachment #8829274 - Flags: approval-mozilla-aurora?
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8829274 [details]
Bug 1332952 - Implement GetAsSourceSurface() for SharedRGBImage.

Looks like a long standing regression, may not affect too many people but let's go for it in aurora 53.   If you would like this fix in beta and think it's worth uplift, it isn't too late.
Attachment #8829274 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
No longer depends on: 1333335
I managed to reproduce this issue on Firefox 53.0a1 (2017-01-21), under Windows 7x64, using the examples from Comment 0.
The issue is no longer reproducible on Firefox 53.0b2, or on Firefox 54.0a2 (2017-03-15).
Tests were performed under Windows 7x64, Mac OS X 10.12.1, Ubuntu 16.04x64.
You need to log in before you can comment on or make changes to this bug.