Use gralloc buffers for WebRTC local camera preview

RESOLVED FIXED

Status

()

defect
P2
major
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: diego, Assigned: sotaro)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(feature-b2g:2.2+)

Details

(Whiteboard: [mobile app][blocking][leave-open])

Attachments

(2 attachments, 16 obsolete attachments)

14.27 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
7.01 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
Reporter

Description

5 years ago
The hardware composer can improve the power efficiency of the Loop mobile client. It is used in the Camcorder and Video apps for this reason.
Reporter

Updated

5 years ago
Blocks: 1036490
Reporter

Updated

5 years ago
Blocks: 1011683
Reporter

Comment 1

5 years ago
STR:

1. Go to Settings -> Developer -> Developer HUD.
2. Enable Developer HUD and "Frames per second"
3. Open Loop Mobile.
4. If HWC is used you will see "HWComposer: FPS is 30" messaged.
5. If HWC is *not* used you will see the framerate displayed on the top-left corner of the screen only
Reporter

Comment 2

5 years ago
Sotaro,

I noticed even though the camera app and video app are both using HWC, a simple app with one textbox and one button cannot be use HWC. I can make it use HWC if I force Tiling off ("Developer" -> "Tiling" -> uncheck).

Is tiling used in most cases now?

How come it doesn't affect the camera app and video app?
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 3

5 years ago
(In reply to Diego Wilson [:diego] from comment #2)
> Sotaro,
> 
> I noticed even though the camera app and video app are both using HWC, a
> simple app with one textbox and one button cannot be use HWC. I can make it
> use HWC if I force Tiling off ("Developer" -> "Tiling" -> uncheck).
> 
> Is tiling used in most cases now?

Yes. If application has a scrollable layer, the layer is created as tiling layer.

> How come it doesn't affect the camera app and video app?

Tiling is used only for scrollable layers on b2g. If layout identify a scrollable layer, it is created by ClientLayerManaer as tile layer.
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/ClientThebesLayer.cpp#156

In camera and video case, application is created as non scrollable.
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 4

5 years ago
> 
> Tiling is used only for scrollable layers on b2g. If layout identify a
> scrollable layer, it is created by ClientLayerManaer as tile layer.
> http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/
> ClientThebesLayer.cpp#156
> 
> In camera and video case, application is created as non scrollable.

Even in video app, at scrollable part, tiled layers are used.
Reporter

Comment 5

5 years ago
Yeah, the video app selection screen does not use HWC.

Do you know what's the simplest way to make an app non-scrollable?
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 6

5 years ago
(In reply to Diego Wilson [:diego] from comment #5)
> Yeah, the video app selection screen does not use HWC.
> 
> Do you know what's the simplest way to make an app non-scrollable?

BenWa, can you answer the question?
Flags: needinfo?(sotaro.ikeda.g) → needinfo?(bgirard)
I believe using overflow: hidden on frames that get unwanted scrolling.
Flags: needinfo?(bgirard)
This issue still occurs on the latest 2.0 build with the v165 KK base. The FPS counter does not show that HWComposer is being used.

Environmental Variables:
Device: Flame 2.0
BuildID: 20140820030000
Gaia: 806d37a264743e04a3e1493136486f3e00124e1e
Gecko: 6329352ca531b977979451e77e5862af485388b2
Version: 32.0 (2.0) 
Firmware Version: v165
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Loop version: 8c71431
(In reply to Diego Wilson [:diego] from comment #5)
> Do you know what's the simplest way to make an app non-scrollable?

Finding what is scrollable and making sure that it matches the size of the parent. A hacky fallback is to use overflow: hidden;. Playing with the app manager's inspector is a good way to find what is causing what.
Whiteboard: [mobile app][blocking]
Fairly sure this should be under FFOS product.
Component: Client → Gaia::Loop
Product: Loop → Firefox OS
Severity: normal → major
Priority: -- → P2
(In reply to Diego Wilson [:diego] from comment #1)
> STR:
> 
> 1. Go to Settings -> Developer -> Developer HUD.
> 2. Enable Developer HUD and "Frames per second"
> 3. Open Loop Mobile.
> 4. If HWC is used you will see "HWComposer: FPS is 30" messaged.
> 5. If HWC is *not* used you will see the framerate displayed on the top-left
> corner of the screen only

Hi Diego! I can not see step number 4 even using the video/camera apps, when recording or playing a previously recorded video. Im using FirefoxOS 2.0 in a Flame, and I've played forcing Tiling on/off but no result in my screen... any tip to check this? Thanks!
Flags: needinfo?(dwilson)
Reporter

Comment 12

5 years ago
Borja,

I can see the HWC messages in logcat on my 8x10 v2.0 build with the camera and video apps.

Sorry, maybe it wasn't clear in the instructions but the "HWComposer:FPS" messages will show in logcat. Try |adb logcat Gecko:D *:S|
Flags: needinfo?(dwilson) → needinfo?(borja.bugzilla)
Whiteboard: [mobile app][blocking] → [mobile app][blocking][feedback-requested]
Whiteboard: [mobile app][blocking][feedback-requested] → [mobile app][blocking]
Hi Diego! I've created the following app:
https://github.com/borjasalguero/firefox-loop-reference
So we could try different libraries (even a simple gUM request) in order to check if HWComposer is working. However I had no FPS info in any case...

Diego, it seems to be a Gecko issue, Do you know where the issue is? Thanks!
Flags: needinfo?(borja.bugzilla) → needinfo?(dwilson)
Reporter

Comment 14

5 years ago
I played around with it. Turns out the culprit is the local video preview. It seems it is not backed by a gralloc buffer. If I disable the local preview and just display the remote video then HWC kicks in.

I suspect the local stream obtained by getUserMedia has never supported gralloc.

Sotaro, this probably involves both the graphics and the media team. Do you know who can help use here?
Flags: needinfo?(dwilson) → needinfo?(sotaro.ikeda.g)
NI to mikeh who handles a lot of the B2G camera issues.
Flags: needinfo?(mhabicher)
Can someone post the layer tree dump for the app?
Remove ni? after IRC discussion.
Flags: needinfo?(mhabicher)
Assignee

Comment 18

5 years ago
(In reply to Diego Wilson [:diego] from comment #14)
> Sotaro, this probably involves both the graphics and the media team. Do you
> know who can help use here?

I am going to check what caused the problem.
Flags: needinfo?(sotaro.ikeda.g)
Sotaro: Check the discussion in http://logs.glob.uno/?c=mozilla%23media&s=15+Sep+2014&e=15+Sep+2014#c115957

Basically, getUserMedia on B2G always calls MediaEngineWebRTCVideoSource::RotateImage().  This copies the data into a CPU memory buffer.  It is then fed to either a PeerConnection (webrtc call), MediaStreamGraph for display in <video>, or both.  (In this case, I believe the app has a local video display, so it would be both).

Far-end video (receiver side) comes in via PeerConnection and MediaPipeline and is inserted into MediaStreamGraph via MediaPipelineReceiveVideo::PipelineListener::RenderVideoFrame() which allocates a GRALLOC_PLANAR_YCBCR buffer.

There was work on bug 976397 to try to pass rotation info with the buffer and avoid CPU rotates - however, other items need it rotated (PeerConnection, MediaRecorder, etc).  I suggest in the IRC log that we could do the rotation "lazily" as needed.
Assignee

Comment 20

5 years ago
Thanks!
Assignee

Comment 21

5 years ago
(In reply to Diego Wilson [:diego] from comment #14)
> I suspect the local stream obtained by getUserMedia has never supported
> gralloc.

The app also needs "overflow: hidden" as in Comment 7. Otherwise, the application have tiled layers. On flame, the following change just remove the tiled layers.

> html {
>  font-size: 15px;
>   overflow: hidden;
> }
Attachment #8490855 - Flags: review?(sotaro.ikeda.g)
Assignee

Comment 23

5 years ago
Comment on attachment 8490855 [details] [diff] [review]
use Gralloc buffer for target of gum image Rotation

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

Looks good!
Attachment #8490855 - Flags: review?(sotaro.ikeda.g) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e87724022bc1

Marking leave-open since per sotaro's comments app changes may be needed as well
Whiteboard: [mobile app][blocking] → [mobile app][blocking][leave-open]
Reporter

Comment 25

5 years ago
(In reply to Randell Jesup [:jesup] from comment #22)
> Created attachment 8490855 [details] [diff] [review]
> use Gralloc buffer for target of gum image Rotation

This crashes for me on v2.0 when it's allocating.
Backed out (on b2g-inbound) since it breaks the feature on real hardware.
https://hg.mozilla.org/integration/b2g-inbound/rev/c3720dfde313
The problem is simple; AllocateAndGetNewBuffer() is fails on Gralloc buffers because there's no recycler.

However, solving that by allocating a memory buffer directly fixes getUserMedia, but breaks video through PeerConnections - likely because of a bug in the Gralloc buffer handling in MediaPipeline.cpp NewData().  (Chroma is messed up; probably some form of I420/NV21 mismatch).
works for getUserMedia/local-self-image, but has chroma problems when fed into MediaPipeline.cpp NewData() for the webrtc encoder/etc
Assignee

Comment 33

5 years ago
ttachment 8491246 write video frame to gralloc buffer as YV12. But MediaPipelineTransmit::PipelineListener::ProcessVideoChunk() handles it as NV21

http://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp#1184
Assignee

Comment 34

5 years ago
This patch is created based on attachment 8491246 [details] [diff] [review]. This patch removed extra copy from it.
Assignee

Comment 35

5 years ago
By the way, TextureClient's creation/deletion and gralloc buffer allocation could cost to cpu. It might be better to recycle it in a next step.
Assignee

Comment 36

5 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #35)
> By the way, TextureClient's creation/deletion and gralloc buffer allocation
> could cost to cpu. It might be better to recycle it in a next step.

It might not be a capability for b2g-v2.0.
Assignee

Comment 39

5 years ago
This is a temporary patch that created based on the following. It fixes chroma problem by temporary hack.

attachment 8491246 [details] [diff] [review]
http://pastebin.mozilla.org/6552547
Assignee

Comment 40

5 years ago
The patch is created based on the following patch. From [1], it removed redundant video frame copy. Confirmed that video is correctly delivered to webrtc peer by using flame JB.

- [1] attachment 8492433 [details] [diff] [review]
- [2] http://pastebin.mozilla.org/6552444
Attachment #8491791 - Attachment is obsolete: true
Attachment #8492433 - Attachment is obsolete: true
Assignee

Comment 41

5 years ago
attachment 8492488 [details] [diff] [review] address's android's YV12 color format's stride problem by ad-hoc way. Need to make clear how to handle it.

Android YV12 have an additional stride's restriction.
  http://androidxref.com/4.4.4_r1/xref/system/core/include/system/graphics.h#92
Assignee

Comment 42

5 years ago
Support of YV12 needs Bug 1060811. Otherwise, taking screenshot cause an application process crash.
Depends on: 1060811
Assignee

Comment 43

5 years ago
The patch is created based on attachment 8492488 [details] [diff] [review]. Add TextureClient recyling. The recycling is a temporary implementation.
Assignee

Updated

5 years ago
Assignee: nobody → sotaro.ikeda.g
Assignee

Updated

5 years ago
Depends on: 1091777
feature-b2g: --- → 2.2+
Changing to Core->Graphics at least until the graphics team is done with this.
Component: Gaia::Loop → Graphics
Product: Firefox OS → Core
Assignee

Updated

5 years ago
Attachment #8491659 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Attachment #8491246 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Attachment #8490855 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Attachment #8492488 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Attachment #8492781 - Attachment is obsolete: true
Assignee

Comment 48

5 years ago
some code clean up.
Attachment #8523225 - Attachment is obsolete: true
Whiteboard: [mobile app][blocking][leave-open] → [mobile app][blocking][leave-open][2.2-feature-qa+]
QA Whiteboard: [2.2-feature-qa+]
Whiteboard: [mobile app][blocking][leave-open][2.2-feature-qa+] → [mobile app][blocking][leave-open]
Sotaro, is this ready for review?  Who's the right person to review it?
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 50

5 years ago
Not ready for review. I need to add one thing. Release gralloc buffer when MediaCodec are released. I am going to work for it next week.
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Updated

5 years ago
Depends on: 1109226
Assignee

Comment 51

5 years ago
Posted patch part 1 - Change to WebRTC (obsolete) — Splinter Review
Attachment #8524035 - Attachment is obsolete: true
Assignee

Comment 52

5 years ago
Posted patch patch - Change to gfx layers (obsolete) — Splinter Review
Assignee

Updated

5 years ago
Attachment #8533888 - Flags: review?(nical.bugzilla)
Assignee

Updated

5 years ago
Attachment #8533887 - Flags: review?(rjesup)
Comment on attachment 8533887 [details] [diff] [review]
part 1 - Change to WebRTC

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

r+, please decide if it's easy enough to fix the odd sizes part or defer with a bug and comment pointing to it

::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp
@@ +587,1 @@
>    uint32_t size = aWidth * aHeight * 3 / 2;

If it's not too hard (just adjusting size here and below), for example), we should fix that now (though b2g is much less likely to get an odd size, if it ever supports tab-sharing it may).  Otherwise, file a followup.

@@ +604,5 @@
>      dstWidth = aWidth;
>      dstHeight = aHeight;
>    }
>  
>    uint32_t half_width = dstWidth / 2;

like here

@@ +625,5 @@
> +  uint8_t* dstPtr = static_cast<uint8_t*>(destMem);
> +
> +  int32_t yStride = destBuffer->getStride();
> +  // Align to 16 bytes boundary
> +  int32_t uvStride = ((yStride / 2) + 15) & ~0x0F;

and here

@@ +631,3 @@
>    libyuv::ConvertToI420(srcPtr, size,
> +                        dstPtr, yStride,
> +                        dstPtr + (yStride * dstHeight + (uvStride * dstHeight / 2)), uvStride,

and here

::: dom/media/webrtc/MediaEngineGonkVideoSource.h
@@ +14,5 @@
>  
>  #include "mozilla/Hal.h"
>  #include "mozilla/ReentrantMonitor.h"
>  #include "mozilla/dom/File.h"
> +#include "mozilla/layers/TextureClientRecycleAllocator.h" 

space on end

::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ +1059,5 @@
>      MOZ_ASSERT(PR_FALSE);
>      return kMediaConduitMalformedArgument;
>    }
>  
> +  // NOTE: udpate when common_types.h changes

update

::: media/webrtc/trunk/webrtc/common_video/libyuv/webrtc_libyuv.cc
@@ +245,5 @@
> +  if (src_video_type == kYV12) {
> +    // In gralloc buffer, yv12 color format's cb and cr's strides are aligned
> +    // to 16 Bytes boundary. See /system/core/include/system/graphics.h
> +    int stride_y = src_width;
> +    int stride_uv = ((stride_y / 2) + 15) & ~0x0F;

((stride_y+1)/2) I think - we do get odd widths and heights in webrtc screensharing

@@ +248,5 @@
> +    int stride_y = src_width;
> +    int stride_uv = ((stride_y / 2) + 15) & ~0x0F;
> +    return libyuv::I420Rotate(src_frame,
> +                              stride_y,
> +                              src_frame + (stride_y * src_height) + (stride_uv * src_height / 2),

(src_height+1)/2 I think
Attachment #8533887 - Flags: review?(rjesup) → review+
Comment on attachment 8533888 [details] [diff] [review]
patch - Change to gfx layers

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

::: gfx/layers/client/TextureClient.cpp
@@ +384,5 @@
>    if (texture && texture->AllocateForSurface(aSize, aAllocFlags)) {
>      return texture;
>    }
>  
> +  if (aAllocFlags & ALLOC_NO_FALLBACK_ALLOCATION) {

This means BufferTextureClient will never be used if the ALLOC_NO_FALLBACK_ALLOCATION bit is set.

I suppose that what you intend to do is to only use the first texture type that is compatible and not fallback if allocation fails in which case you want to merge move branch in the one below, something like:

if (texture) { // <-- if we reach this with texture != nullptr and haven't returned yet, means we failed to allocate. 
  NS_WARNING("Failed to allocate a TextureClient");
  if (aAllocFlags & ALLOC_NO_FALLBACK_ALLOCATION) {
    return nullptr;
  }
}

::: gfx/layers/client/TextureClient.h
@@ +70,5 @@
>  enum TextureAllocationFlags {
>    ALLOC_DEFAULT = 0,
>    ALLOC_CLEAR_BUFFER = 1,
> +  ALLOC_CLEAR_BUFFER_WHITE = 2,
> +  ALLOC_NO_FALLBACK_ALLOCATION = 3

TextureAllocationFlags is a bitfield so you want to use a power of two like ALLOC_NO_FALLBACK_ALLOCATION = 4 for it not to overlap with the other fields.
Attachment #8533888 - Flags: review?(nical.bugzilla) → review-
Assignee

Comment 55

5 years ago
> 
> I suppose that what you intend to do is to only use the first texture type
> that is compatible and not fallback if allocation fails in which case you
> want to merge move branch in the one below, something like:
> 
> if (texture) { // <-- if we reach this with texture != nullptr and haven't
> returned yet, means we failed to allocate. 
>   NS_WARNING("Failed to allocate a TextureClient");
>   if (aAllocFlags & ALLOC_NO_FALLBACK_ALLOCATION) {
>     return nullptr;
>   }
> }

In this bugs use case, we do not want to use BufferTextureClient at all on gonk. We want to use only TextureClientOGL.

nical, how do you think about it?
Flags: needinfo?(nical.bugzilla)
Assignee

Comment 56

5 years ago
> 
> ::: gfx/layers/client/TextureClient.h
> @@ +70,5 @@
> >  enum TextureAllocationFlags {
> >    ALLOC_DEFAULT = 0,
> >    ALLOC_CLEAR_BUFFER = 1,
> > +  ALLOC_CLEAR_BUFFER_WHITE = 2,
> > +  ALLOC_NO_FALLBACK_ALLOCATION = 3
> 
> TextureAllocationFlags is a bitfield so you want to use a power of two like
> ALLOC_NO_FALLBACK_ALLOCATION = 4 for it not to overlap with the other fields.

Will update in next patch.
(In reply to Sotaro Ikeda [:sotaro] from comment #55)
> In this bugs use case, we do not want to use BufferTextureClient at all on
> gonk. We want to use only TextureClientOGL.

Then I would prefer to name it something more explicit like DISALLOW_BUFFERTEXTURECLIENT. Otherwise we'll forget about this and a year from now we'll probably think that DISALLOW_FALLBACK not using BufferTextureClient (which is not a fallback on, say, android) is a bug.
But actually in this specific case, I don't think we should use a "factory"-like method since we know exactly what kind of TextureClient we want (gralloc). Instead, I think that GrallocImage should just create a GrallocTextureClient directly rather than ask for a something and hope to get a gralloc texture.
Would this work for you?
Flags: needinfo?(nical.bugzilla)
Assignee

Comment 58

5 years ago
The patch uses TextureClientRecycleAllocator as Texture Recycling. I do not think that GrallocTextureClient creation via GrallocImage do not fit this purpose. And I do not want code duplication of TextureClientRecycleAllocatorImp::CreateOrRecycleForDrawing(). DISALLOW_BUFFERTEXTURECLIENT seem better for it.

http://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/TextureClientRecycleAllocator.cpp#130
Assignee

Comment 59

5 years ago
Posted patch patch - Change to gfx layers (obsolete) — Splinter Review
Apply the comments.
Attachment #8533888 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Attachment #8534368 - Flags: review?(nical.bugzilla)
Assignee

Comment 60

5 years ago
In current patch, TextureClientRecycleAllocator's recycling is not enough. Because of it local camera preview has a lot of lag. It needs to be addressed.
Assignee

Updated

5 years ago
Attachment #8534368 - Flags: review?(nical.bugzilla)
Assignee

Comment 61

5 years ago
From Bug 1109226 Comment 4, it becomes clear that even when this bug is fixed, even when this bug is addressed, the Loop client does not use Hwc during WebRTC.

The Loop client clip local video preview to round shape. It make ImageLayer to use MaksLayer. We can not use HWC, when Layer has MaksLayer.
Assignee

Comment 62

5 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #60)
> In current patch, TextureClientRecycleAllocator's recycling is not enough.
> Because of it local camera preview has a lot of lag. It needs to be
> addressed.

When I increased the TextureClientRecycleAllocator's pool size from 2 to 10. WebRTC becomes soother than without the patch. fPs on compositor seems to rise from 20 to 30.
Assignee

Comment 63

5 years ago
(In reply to Randell Jesup [:jesup] from comment #53)
> Comment on attachment 8533887 [details] [diff] [review]
> part 1 - Change to WebRTC
> 
> Review of attachment 8533887 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+, please decide if it's easy enough to fix the odd sizes part or defer
> with a bug and comment pointing to it
> 
> ::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp
> @@ +587,1 @@
> >    uint32_t size = aWidth * aHeight * 3 / 2;
> 
> If it's not too hard (just adjusting size here and below), for example), we
> should fix that now (though b2g is much less likely to get an odd size, if
> it ever supports tab-sharing it may).  Otherwise, file a followup.

It seems better to split to a different bug to simplify the bug. I will create a follow up bug.

> 
> @@ +604,5 @@
> >      dstWidth = aWidth;
> >      dstHeight = aHeight;
> >    }
> >  
> >    uint32_t half_width = dstWidth / 2;
> 
> like here
> 
> @@ +625,5 @@
> > +  uint8_t* dstPtr = static_cast<uint8_t*>(destMem);
> > +
> > +  int32_t yStride = destBuffer->getStride();
> > +  // Align to 16 bytes boundary
> > +  int32_t uvStride = ((yStride / 2) + 15) & ~0x0F;
> 
> and here
> 
> @@ +631,3 @@
> >    libyuv::ConvertToI420(srcPtr, size,
> > +                        dstPtr, yStride,
> > +                        dstPtr + (yStride * dstHeight + (uvStride * dstHeight / 2)), uvStride,
> 
> and here
> 
> ::: dom/media/webrtc/MediaEngineGonkVideoSource.h
> @@ +14,5 @@
> >  
> >  #include "mozilla/Hal.h"
> >  #include "mozilla/ReentrantMonitor.h"
> >  #include "mozilla/dom/File.h"
> > +#include "mozilla/layers/TextureClientRecycleAllocator.h" 
> 
> space on end

I will update in a next patch.

> 
> ::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
> @@ +1059,5 @@
> >      MOZ_ASSERT(PR_FALSE);
> >      return kMediaConduitMalformedArgument;
> >    }
> >  
> > +  // NOTE: udpate when common_types.h changes
> 
> update

Fix in a next patch.

> 
> ::: media/webrtc/trunk/webrtc/common_video/libyuv/webrtc_libyuv.cc
> @@ +245,5 @@
> > +  if (src_video_type == kYV12) {
> > +    // In gralloc buffer, yv12 color format's cb and cr's strides are aligned
> > +    // to 16 Bytes boundary. See /system/core/include/system/graphics.h
> > +    int stride_y = src_width;
> > +    int stride_uv = ((stride_y / 2) + 15) & ~0x0F;
> 
> ((stride_y+1)/2) I think - we do get odd widths and heights in webrtc
> screensharing

Yhea, I will update in a next patch.

> 
> @@ +248,5 @@
> > +    int stride_y = src_width;
> > +    int stride_uv = ((stride_y / 2) + 15) & ~0x0F;
> > +    return libyuv::I420Rotate(src_frame,
> > +                              stride_y,
> > +                              src_frame + (stride_y * src_height) + (stride_uv * src_height / 2),
> 
> (src_height+1)/2 I think

I will update in a next patch.
Assignee

Comment 64

5 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #62)
> (In reply to Sotaro Ikeda [:sotaro] from comment #60)
> > In current patch, TextureClientRecycleAllocator's recycling is not enough.
> > Because of it local camera preview has a lot of lag. It needs to be
> > addressed.
> 
> When I increased the TextureClientRecycleAllocator's pool size from 2 to 10.
> WebRTC becomes soother than without the patch. fPs on compositor seems to
> rise from 20 to 30.

It seems like my misunderstanding. Next time I tested on master flame, fps is almost same.
Assignee

Updated

5 years ago
Blocks: 1109957
Assignee

Comment 65

5 years ago
Apply the comments. Carry "r=jesup".
Attachment #8533887 - Attachment is obsolete: true
Assignee

Comment 66

5 years ago
Attachment #8534368 - Attachment is obsolete: true
Assignee

Comment 67

5 years ago
Comment on attachment 8534711 [details] [diff] [review]
patch part 1 - Change to WebRTC

Carry "r=jesup".
Attachment #8534711 - Flags: review+
Assignee

Updated

5 years ago
Attachment #8534712 - Flags: review?(nical.bugzilla)
Attachment #8534712 - Flags: review?(nical.bugzilla) → review+
Assignee

Updated

5 years ago
Depends on: 1110962
Assignee

Comment 69

5 years ago
Add failure checks. Carry "r=jesup".
Attachment #8534711 - Attachment is obsolete: true
Attachment #8536591 - Flags: review+
Assignee

Comment 71

5 years ago
Fix incorrect assertion. Carry "r=nical".
Attachment #8534712 - Attachment is obsolete: true
Attachment #8536696 - Flags: review+
Flags: in-moztrap?(echang)
Assignee

Updated

5 years ago
Summary: Loop client does not render with Hardware accelerated composer (HWC) → Use gralloc buffers for WebRTC local camera preview
Assignee

Comment 75

5 years ago
Changed Summary as to correct name. This bug does not about to ensure HWC usage.
Assignee

Comment 76

5 years ago
(In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #75)
> Changed Summary as to correct name. This bug does not about to ensure HWC
> usage.

Even when this bug is fixed. Loop applicaiton does not use HWC because of loop application's implementation. See Comment 61.
Assignee

Comment 77

5 years ago
By using current Loop application, only b2g devices with overlay HWC has a possibility to use HWC. It is going to be handled by Bug 1110962.
Assignee

Updated

5 years ago
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Flags: in-moztrap?(echang) → in-moztrap-

Updated

4 years ago

Updated

4 years ago
No longer blocks: CAF-v3.0-FL-metabug
You need to log in before you can comment on or make changes to this bug.