Closed Bug 1527951 Opened 8 months ago Closed 8 months ago

Assertion failure: mFormat == SurfaceFormat::B8G8R8A8 || src[3] == 0xFF

Categories

(Core :: ImageLib, defect, P3)

65 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- wontfix
firefox66 --- fixed
firefox67 --- verified

People

(Reporter: bc, Assigned: aosmond)

References

(Blocks 1 open bug, )

Details

(Keywords: assertion, regression)

Attachments

(1 file)

  1. https://www.goodsearch.org/

  2. Assertion failure: mFormat == SurfaceFormat::B8G8R8A8 || src[3] == 0xFF, at z:/build/build/src/image/decoders/nsWebPDecoder.cpp:492
    #01: mozilla::image::nsWebPDecoder::ReadHeader(WebPDemuxer *,bool) [image/decoders/nsWebPDecoder.cpp:399]
    #02: mozilla::image::nsWebPDecoder::ReadData() [image/decoders/nsWebPDecoder.cpp:0]
    #03: mozilla::image::nsWebPDecoder::UpdateBuffer(mozilla::image::SourceBufferIterator &,mozilla::image::SourceBufferIterator::State) [image/decoders/nsWebPDecoder.cpp:0]
    #04: mozilla::image::nsWebPDecoder::DoDecode(mozilla::image::SourceBufferIterator &,mozilla::image::IResumable *) [image/decoders/nsWebPDecoder.cpp:119]
    #05: mozilla::image::Decoder::Decode(mozilla::image::IResumable *) [image/Decoder.cpp:125]
    #06: mozilla::image::DecodedSurfaceProvider::Run() [image/DecodedSurfaceProvider.cpp:125]
    #07: mozilla::image::DecodePoolWorker::Run() [image/DecodePool.cpp:266]
    #08: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1149]

Windows/Linux. From the original webp landing. Doesn't crash opt from what I can tell.

Summary: Assertion failure: mFormat == SurfaceFormat::B8G8R8A8 || src[3] == 0x → Assertion failure: mFormat == SurfaceFormat::B8G8R8A8 || src[3] == 0xFF
Assignee: nobody → aosmond
Priority: -- → P3

The assertion itself is probably harmless, but I think we can trigger release asserts in skia because of this. We should just ignore the alpha channel from the decoder if the surface is BGRX.

(In reply to Andrew Osmond [:aosmond] from comment #2)

The assertion itself is probably harmless, but I think we can trigger release asserts in skia because of this. We should just ignore the alpha channel from the decoder if the surface is BGRX.

With WebRender, it causes us to be completely transparent on Linux -- it shows the windows/desktop underneath the image. Very confusing!

Pushed by aosmond@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c887a569595
Ignore WebP image's alpha channel when a frame is marked as opaque. r=tnikkel
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

I'm assuming this can ride the trains, but feel free to nominate for Beta approval if there's a user impact which justifies consideration.

Flags: in-testsuite+

Comment on attachment 9044293 [details]
Bug 1527951 - Ignore WebP image's alpha channel when a frame is marked as opaque.

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1294490
  • User impact if declined: May experience rare (release assertion) crash or visual distortion with malformed WebP images.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The change is not risky because we now explicitly set the alpha channel to be opaque instead of relying upon libwebp to give us a sane value. It produces more restrictive output than before, and is thus more predictable. I also added tests which explicitly test this condition.
  • String changes made/needed: N/A
Attachment #9044293 - Flags: approval-mozilla-beta?

I can verify this works as expected in nightly since the patch landed. Images like:

https://hg.mozilla.org/integration/mozilla-inbound/raw-file/1c887a569595/image/test/gtest/transparent-no-alpha-header.webp

should be all black now, which it is.

Comment on attachment 9044293 [details]
Bug 1527951 - Ignore WebP image's alpha channel when a frame is marked as opaque.

Fix verified in nightly, has test coverage.
OK for uplift for beta 12.

Attachment #9044293 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.