1015332 - Youtube video playback in Browser is not using H/W composition.

Status: RESOLVED DUPLICATE of bug 988511

(Core :: Graphics: Layers, defect, P2)

Description

[Sushil]

Youtube video playback in Browser is not using H/W Composition. It is falling back to GPU Composition due to NULL gralloc buffer for Thebes layer. Here is log:

HWC layer[0]::ColorLayerComposite: dst=[0 0 1080 1920] src=[0 0 1920  1080]
E/HWComposer( 217): ThebesLayer Layer doesn't have a gralloc buffer

GPU Composition impacts power numbers in this use case. Can you please check why the gralloc buffer is NULL in this use case ? Video playback in Video app is fine and using H/W Composition.

Comment 1

[Sotaro Ikeda [:sotaro]]

This might be dup of Bug 988851.

Comment 2

[Andreas Gal :gal]

Is the layer scrollable? If so we are using tiling, and H/W composition is not possible. YT video playback in the browser has traditionally not been a test case we have used for CS. Does Chrome use H/W composition in this case?

Comment 3

[Sushil]

How to disable tiled rendering, just to verify if it uses H/W Composition ? It seems Bug 988851 has been re-opened (from dup). BTW, it is also happening on full screen landscape 720p Video in Youtube on Browser, which should not be scrollable since it is full screen and should be the only layer in frame.

Comment 4

[Preeti Raghunath(:Preeti)]

Sotaro,

Is this a regression? Power impact here?

Comment 5

[Preeti Raghunath(:Preeti)]

Milan

Can we please respond to comment 3?

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Sushil from comment #3)
> How to disable tiled rendering, just to verify if it uses H/W Composition ?

The option is a true/false, layers.enable-tiles, true by default.  You should also see it in the developer menu if you can run the phone first, then change the value.

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

This is really bug 1006083, which is bug 988511.

Comment 8

[Sotaro Ikeda [:sotaro]]

(In reply to Preeti Raghunath(:Preeti) from comment #4)
> Sotaro,
> 
> Is this a regression? Power impact here?

It could make some regression to the power consumption. But I am not sure the regression could have a value to be "b2g v1.4+". There was a similar bug about camera. It is Bug 995680.

Comment 9

[Sotaro Ikeda [:sotaro]]

(In reply to Preeti Raghunath(:Preeti) from comment #4)
> Sotaro,
> 
> Is this a regression?

As in bug 988511, on b2g v1.3, on youtube video fullscreen playback, hw composer was used.

Comment 10

[Preeti Raghunath(:Preeti)]

Mike

Please weigh in on regression on power consumption

Comment 11

[Mason Chang [Inactive] [:mchang]]

From Triage: Dependent bug 988511, comment #7 - https://bugzilla.mozilla.org/show_bug.cgi?id=988511#c7 - This is not a blocker because the power usage from the network should use more than the GPU.

Comment 12

[Inder]

The logic that network probably uses more power so it's okay to use GPU instead of using HWC doesn't make sense. It's based on assumption and also this is such a common use case so we should try to optimize on power consumption in all aspects. We should definitely try to fix it at least for 2.0 and if possible in v1.4 as well.

Comment 13

[Preeti Raghunath(:Preeti)]

Milan

Please prioritize for 2.0.

Comment 14

[Milan Sreckovic [:milan] (needinfo for best results)]

I'm assuming bug 988511 is the first priority - YouTube videos in full screen.

Comment 15

[Sushil]

In HwcComposer2D, if we mark the layer having NULL gralloc buffer as HWC_SKIP_LAYER instead of forcing the entire frame to use GPU Composition, we can still use HWC for other layers in the frame. We just checked & verified that full screen and non-full screen Youtube video playback is using HWC by doing this. This will save power during Youtube video playback. I will upload the patch.

Meanwhile, please debug why HwcComposer2D is getting NULL gralloc buffer when tiling is enabled (Bug 988511).

Comment 16

[Sushil]

Attachment: Optimize frame having layers with NULL gralloc buffer.

On platforms with partial HWC Composition support, mark the layers with NULL gralloc buffer, as HWC_SKIP_LAYER, to have them composed by GPU, so remaining layers get composed by HWC. It gives an opportunity to save power, by avoiding fallback to GPU Composition entirely.

Comment 17

[Sushil]

Comment on attachment 8432053 [details] [diff] [review]
Optimize frame having layers with NULL gralloc buffer.

Adding Sotaro to review the patch.

Comment 18

[Diego Wilson [:diego]]

Will it fall back to full GPU if it doesn't support partial HWC composition?

Comment 19

[Sushil]

Yes, HAL will take the decision whether the frame can take partial HWC Composition path or full GPU Composition. By marking as HWC_SKIP_LAYER, we are telling HWC that do not compose this layer, it will be composed by GPU. And ANDROID_VERSION check is needed since ICS based HALs do not support partial HWC Composition.

Comment 20

[Diego Wilson [:diego]]

Comment on attachment 8432053 [details] [diff] [review]
Optimize frame having layers with NULL gralloc buffer.

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

Hijacking r? from Sotaro. LGTM

Comment 21

[Sotaro Ikeda [:sotaro]]

(In reply to Sushil from comment #17)
> Comment on attachment 8432053 [details] [diff] [review]
> Optimize frame having layers with NULL gralloc buffer.
> 
> Adding Sotaro to review the patch.

The patch looks just skip tiled layer. How tiled layer is drawn?

Comment 22

[Sushil]

> 
> The patch looks just skip tiled layer. How tiled layer is drawn?

Tiled layer is drawn by GPU. We are just asking HWC hal not to compose this layer by marking it as HWC_SKIP_LAYER. Meanwhile, you will still need to debug (in Bug 988511) why the gralloc buffer handle is NULL when tiling is enabled. And fortunately, in YouTube use case, all layers other than Video layer do not change most of the times, so we land-up in using cache-based partial HWC Composition which saves lot of power. And in full screen landscape YouTube, the tiled layer is actually behind the full screen opaque Video layer, so only Video layer gets composed by HWC, which again saves power by avoiding GPU entirely.

Comment 23

[Sotaro Ikeda [:sotaro]]

Sushi, can you explain more concretely about by which code the tiled layer is rendered by gpu?

Comment 24

[Sushil]

Sotaro,

Let's say YouTube frame has 2 layers:
Layer[0]: Tiled layer, with null gralloc buffer. We mark it HWC_SKIP_LAYER, comp type = HWC_FRAMEBUFFER
Layer[1]: Video layer, comp type = HWC_FRAMEBUFFER

In mHwc->prepare(), HAL does not touch Layer[0] but change composition type of Layer[1] to HWC_OVERLAY since it can compose the Video layer.

After mHwc->prepare(), [1] and [2] takes care of letting GPU render the Layer[0] on FB layer, then we get the Render() call at [3] which includes eglSwapBuffers() call via [4] followed by Commit() call at [5], hence both FB layer (which has the content composed by GPU) and Layer[1] are now drawn by HWC in mHwc->set(). This is called partial HWC Composition path.

[1]: http://mxr.mozilla.org/mozilla-central/source/widget/gonk/HwcComposer2D.cpp#557
[2]: http://mxr.mozilla.org/mozilla-central/source/widget/gonk/HwcComposer2D.cpp#582
[3]: http://mxr.mozilla.org/mozilla-central/source/widget/gonk/HwcComposer2D.cpp#606
[4]: http://mxr.mozilla.org/mozilla-central/source/widget/gonk/HwcComposer2D.cpp#613
[5]: http://mxr.mozilla.org/mozilla-central/source/widget/gonk/HwcComposer2D.cpp#639

Comment 25

[Sotaro Ikeda [:sotaro]]

Thanks for the explanation! I understand it.

Comment 26

[Sushil]

Attachment: Optimize frame having layers with NULL gralloc buffer. r=dwilson

Uploading HG friendly version of the reviewed patch.

Comment 27

[Sushil]

Sotaro, can you please run the patch: attachment 8433632 [details] [diff] [review] on Try server and share the link ?

Comment 28

[Sotaro Ikeda [:sotaro]]

(In reply to Sushil from comment #27)
> Sotaro, can you please run the patch: attachment 8433632 [details] [diff] [review]
> [review] on Try server and share the link ?

Ok. I am going push to tryserver.

Comment 29

[Sotaro Ikeda [:sotaro]]

https://tbpl.mozilla.org/?tree=Try&rev=45aafb02841e

Comment 30

[Sushil]

Try server link: https://tbpl.mozilla.org/?tree=Try&rev=45aafb02841e

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a4cde9c1130

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2a4cde9c1130

Comment 33

[Preeti Raghunath(:Preeti)]

Already in 2.0. Not taking it in 1.4

Comment 34

[bhargavg1]

(In reply to Preeti Raghunath(:Preeti) from comment #33)
> Already in 2.0. Not taking it in 1.4

Any reason not to take for 1.4, this gives power savings which would help in battery life

Comment 35

[Sushil]

Power savings in YouTube on Browser, which is a popular use-case.

Comment 36

[bhargavg1]

ni Preeti as to consider for 1.4

Comment 37

[Preeti Raghunath(:Preeti)]

(In reply to bhargavg1 from comment #36)
> ni Preeti as to consider for 1.4

Please provide risk analysis here as I am concerned with taking too many blockers in 1.4

Comment 38

[Sotaro Ikeda [:sotaro]]

Need to back out the patch. It causes Bug 1020527.

Comment 39

[Jason Smith [:jsmith]]

(In reply to Preeti Raghunath(:Preeti) from comment #37)
> (In reply to bhargavg1 from comment #36)
> > ni Preeti as to consider for 1.4
> 
> Please provide risk analysis here as I am concerned with taking too many
> blockers in 1.4

I think bug 1020527 already answers this question. Way too risky for 1.4 at this point.

Comment 40

[(no longer active)]

Sorry, I had to back this out because of bug 1020527.

https://hg.mozilla.org/mozilla-central/rev/951e3a671279

Comment 41

[Sushil]

Sotaro, since you reproduced the issue on your device, I have couple of questions:

1. How to reproduce that sliding home screen (with scroll bar) ?
2. Is Flame a non-H/W Overlay device and is it using our HAL ?
3. Can you log ANDROID_VERSION number on Flame ?

Comment 42

[Sotaro Ikeda [:sotaro]]

(In reply to Sushil from comment #41)
> Sotaro, since you reproduced the issue on your device, I have couple of
> questions:
> 
> 1. How to reproduce that sliding home screen (with scroll bar) ?

Vertical homescreen seems to set to default on master from today.

> 2. Is Flame a non-H/W Overlay device and is it using our HAL ?

I am not sure about Flame device's detail. But it uses the hal.

> 3. Can you log ANDROID_VERSION number on Flame ?

I am going to check. IIRC, it uses JB. Surface::hook_dequeueBuffer_DEPRECATED() is still used around FramebufferSurface.

Comment 43

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #42)
> (In reply to Sushil from comment #41)
> > Sotaro, since you reproduced the issue on your device, I have couple of
> > questions:
> > 
> > 1. How to reproduce that sliding home screen (with scroll bar) ?
> 
> Vertical homescreen seems to set to default on master from today.

If you uses relatively recent master. You can change homescreen from Settings app. It is under "Homescreen" item.

Comment 44

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #42)
> 
> > 3. Can you log ANDROID_VERSION number on Flame ?
> 

ANDROID_VERSION is 18. build.prop has the following.

 ro.build.version.sdk=18

Comment 45

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #42)
> 
> > 2. Is Flame a non-H/W Overlay device and is it using our HAL ?
> 
> I am not sure about Flame device's detail. But it uses the hal.

flame device's manifest is the following. It get source code from caf. Therefore hal's source is also from caf.
https://github.com/mozilla-b2g/b2g-manifest/blob/master/flame.xml

Comment 46

[Michael Vines [:m1] [:evilmachines]]

Sushil, we could check using b2g_jb_3.3

Comment 47

[Sushil]

I am able to reproduce this issue on v1.4 reference device in Video App in the video list frame (which is scrollable).

E/HWComposer(  229): HWC layer[0]::ThebesLayer: dst=[0 0 480 800] src=[0 0 480 800] hnd=0xaa42fc40
E/HWComposer(  229): COLOR layer: bufferRect = visibleRect = [0 0 480 800]
E/HWComposer(  229): HWC layer[0]::ColorLayerComposite: dst=[0 0 480 800] src=[0 0 480 800] hnd=0x0
E/HWComposer(  229): HWC layer[0]::ThebesLayer: dst=[0 0 480 800] src=[0 0 480 800] hnd=0xac12ed30
E/HWComposer(  229): ThebesLayer Layer doesn't have a gralloc buffer
E/HWComposer(  229): SKIP layer: bufferRect=[0 0 0 0] visibleRect=[0 0 480 2304]
E/HWComposer(  229): PrepareLayerRects failed, return true!!
E/HWComposer(  229): HWC layer[1]::ThebesLayer: dst=[0 740 480 800] src=[0 0 480 60] hnd=0xabfbd560
E/HWComposer(  229): Frame rendered

Since bufferRect is zero rect in case of Tiled layer so PrepareLayerRects() fails and this layer is not being added to HWC layer list and is not considered for Composition at all. The remaining 2 layers are happily composed by HWC. That's why the content is missing on scrollable Home Screen / Video list frame. So this patch is in-complete in terms of supporting Tiled layer in partial HWC Composition.

Now, to handle the tiled layer, there can be 2 approaches:
1. Handle it like Color layer. For this we need to get reliable visibleRect for the tiled layer. If we see above layer configs, visibleRect=[0 0 480 2304] which is way out of bound in terms of height. So if we want to derive buffer rect from visible rect (like Color layer) at [1], gecko layer framework needs to calculate and send correct effective visible region for the tiled layer. Please explain why the visible region is too much out of bounds or is it height of entire buffer, but not tiled layer. Plus other attributes like blending, opacity, transform, etc. should be final (like Color layer) and they should not change later on because they affect composition of HWC composited layers in frame.

2. For a tiled layer, set mSize.width & mSize.height appropriately in RenderState, so we can derive buffer rect from [2]. Currently, they are 0.

This is risky for v1.4. Sotaro, what is status of Bug 988511. Will we get a valid buffer for a tiled layer, once that bug is fixed ?

[1]: http://mxr.mozilla.org/mozilla-central/source/widget/gonk/HwcComposer2D.cpp#284
[2]: http://mxr.mozilla.org/mozilla-central/source/widget/gonk/HwcComposer2D.cpp#292

Comment 48

[Sotaro Ikeda [:sotaro]]

(In reply to Sushil from comment #47)
> This is risky for v1.4. Sotaro, what is status of Bug 988511. Will we get a
> valid buffer for a tiled layer, once that bug is fixed ?

Bug 988511 fix is m-i now. It does not add hw composer support to tiled layers. It suppress tiled layer rendering when youtube is in fullscreen mode.

Comment 49

[Sushil]

I verified the patch of Bug 988511 on v1.4 reference device. Full screen video playback on YouTube is now using HWC in both Landscape and Portrait modes, which should help in power numbers. So I am closing this bug, marking it as dup of Bug 988511.