Closed Bug 1254010 Opened 6 years ago Closed 6 years ago

Investigate scaling during RGB -> YUV conversion with basic compositor

Categories

(Core :: Graphics: Layers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: jrmuizel, Assigned: sotaro)

References

(Blocks 1 open bug)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file, 4 obsolete files)

This can save time and can be faster then our general scaling path. libyuv has scaling code.
Lee's prototype: https://pastebin.mozilla.org/8862899

He gets 57 fps with this vs 54 with cairo and 50 with skia.
Summary: Investigate scaling during RGB -> YUV conversion → Investigate scaling during RGB -> YUV conversion with basic compositor
Whiteboard: [gfx-noted]
So this was the current patch I was using for experimentation.

It depends on the render target's DT to have implemented LockBits. There are certain cases we use BasicCompositor where this is not true and would have to be fixed: DrawTargetCairo with win32 surfaces, and also for unaccelerated DrawTargetCG. Cairo image surfaces and Skia DTs already have this working, although this would not be possible with Cairo/xrender.

The performance benefits seemed somewhat marginal in my test scenarios - a few fps when scaling from 720p to 2880x1620 on a fullscreen youtube video.

Sotaro, do you want to investigate further with this and see if we want to clean this up and integrate it, or otherwise determine if it is not worth it?
Flags: needinfo?(sotaro.ikeda.g)
Depends on: 1255342
Jeff already implemented windows related thing in Bug 1255342.

With patches of Bug 1255320 and Bug 1255303 applied. The following video with 720p was 10 fps with fullscreen.
  https://www.youtube.com/watch?v=e-ORhEE9VVg

By adding patches of Bug 1254010 and Bug 1255342. The video became 12fps with fullscreen. Improvement was 2fps. But it actually improve performance.
Flags: needinfo?(sotaro.ikeda.g)
I head a look at the scaling code in gfx/ycbcr and it doesn't look very while optimized. From my reading it just uses 3 lookup tables per pixel of input.

It should be able to do noticeably better. Perhaps it's worth trying to use libyuv or libswscale from ffmpeg.
Just rebased.
Attachment #8728466 - Attachment is obsolete: true
Just re-based.
Attachment #8758559 - Attachment is obsolete: true
I re-checked fps with current master, since Bug 1254011 is progressed as "Avoid allocating RGB buffer for YUV data everytime". Fps was a bit better than before Bug 1280839 seemed to improve it.

Compared fullscreen youtube playback performance between "master + Bug 1254011" and "master + bug 1254010 + attachment 8767059 [details] [diff] [review] [diff] [review]" by using the following video on my laptop.

 - https://www.youtube.com/watch?v=iNJdPyoqt8U (30fps)

- "master + bug 1254010 + attachment 8761895 [details] [diff] [review] [diff] [review] [diff] [review]" (scaling and conversion once if possible)
  + 2160p(4K): 12 - 30 fps (fps was unstable)
  + 1440p    : 24 fps
  + 1080p    : 30 fps
  + 720p     : 30 fps
  + 480p     : 30 fps

- "master + Bug 1254011" (with recycle buffer and defer YUV to RGB conversion)
  + 2160p(4K): 8 - 23 fps (fps was unstable)
  + 1440p    : 18 fps
  + 1080p    : 24 fps
  + 720p     : 29 fps
  + 480p     : 30 fps

- "master"
  + 2160p(4K): 6 - 16 fps (fps was unstable)
  + 1440p    : 15 fps
  + 1080p    : 21 fps
  + 720p     : 28 fps
  + 480p     : 30 fps
Since Bug 1254011, attachment 8761926 [details] [diff] [review] does not work. It needs to be updated based on it.
Assignee: nobody → sotaro.ikeda.g
Depends on: 1275441
Attachment #8776868 - Flags: review?(jmuizelaar)
Comment on attachment 8776868 [details] [diff] [review]
patch - Scaling during RGB -> YUV conversion with BasicCompositor if possible

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

Can you double check the performance improvement and include it in the commit message?
Attachment #8776868 - Flags: review?(jmuizelaar) → review+
Compared fullscreen youtube playback performance between by using the following video on my laptop.
 - https://www.youtube.com/watch?v=iNJdPyoqt8U (30fps)

- "master + attachment 8776868 [details] [diff] [review]
  + 2160p(4K): 12 - 30 fps (fps was unstable)
  + 1440p    : 22 - 30 fps (fps was unstable)
  + 1080p    : 30 fps
  + 720p     : 30 fps
  + 480p     : 30 fps

- "master"
  + 2160p(4K): 10 - 26 fps (fps was unstable)
  + 1440p    : 16 - 26 fps (fps was unstable)
  + 1080p    : 22 fps
  + 720p     : 28 fps
  + 480p     : 30 fps
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2719a392633d
Scaling during RGB -> YUV conversion with BasicCompositor if possible. basic_compositor_video improved on windows. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/2719a392633d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.