PlanarYCbCrImage::GetAsSurface does not create surface optimized for platform

RESOLVED FIXED in mozilla21

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: romaxa, Assigned: romaxa)

Tracking

unspecified
mozilla21
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
While testing OMTC embedding on Arm device platform I noticed that we do not use arm neon yuv conversion.
we do create surface in PlanarYCbCrImage::GetAsSurface, and mOffscreenFormat always gfxASurface::ImageFormatUnknown.
I guess it would make sense use platform::OffscreenFormat in case if mOffscreenFormat is not set.
(Assignee)

Comment 1

6 years ago
Created attachment 695664 [details] [diff] [review]
Use platform offscreen format when default not set
Attachment #695664 - Flags: review?(nsilva)
(Assignee)

Updated

6 years ago
Attachment #695664 - Flags: review?(joe)
Comment on attachment 695664 [details] [diff] [review]
Use platform offscreen format when default not set

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

I would prefer if GetOffscreenFormat() had that logic in it, actually.

While I'm supportive of this in general, I'm not sure if it's a bug that SetOffscreenFormat() hasn't been called, so I'll leave it up to nical for the final review.
Attachment #695664 - Flags: review?(joe) → review+
Comment on attachment 695664 [details] [diff] [review]
Use platform offscreen format when default not set

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

I agree with Joe, GetOffscreenFormat should have this logic. Actually I wonder why we store mOffscreenFormat as member of PlanarYCbCrImage rather than making GetOffscreenFormat virtual and returning the platform's offscreen format by default, no one seems to set the offscreen format except sub-classes. I'd be happy with either of these two solutions.
(Assignee)

Comment 4

6 years ago
Created attachment 703782 [details] [diff] [review]
Use platform offscreen format when default not set
Assignee: nobody → romaxa
Attachment #695664 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #695664 - Flags: review?(nsilva)
Attachment #703782 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #703782 - Flags: review? → review?(nical.bugzilla)
Comment on attachment 703782 [details] [diff] [review]
Use platform offscreen format when default not set

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

Thanks
Attachment #703782 - Flags: review?(nical.bugzilla) → review+
https://hg.mozilla.org/mozilla-central/rev/cff1c1aa5261
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
(Assignee)

Updated

6 years ago
Depends on: 855586
You need to log in before you can comment on or make changes to this bug.