(Branched from bug 861050) In gfx/layers/client/ImageClient.cpp, in ImageClientSingle::UpdateImage, there is only a path from PLANAR_YCBCR and no path for GRALLOC_PLANAR_YCBCR. But on B2G, we have a GRALLOC_PLANAR_YCBCR. So we don't take any fast path there and fall back to the slow path of calling GetAsSurface and doing things in software.

Thanks for opening this bug, Benoit, and for flagging this as a likely issue with current B2G WebRTC video performance. Benoit & Milan -- Getting a solution to this will be very important to achieving usable video performance on B2G for WebRTC. Can we make this bug a high priority to understand and solve?

That sounds like a reasonable choice of next thing for me to work on. Do you have a testcase for me to test against?

See the patches on bug 861050. You can try simple WebRTC tests like from the webrtc-landing demo page (perhaps http://nightly-gupshup.herokuapp.com/). The pc_test.html page is a heavy test, in that it's a two-way call within the same page (basically two calls at once). For perf monitoring, this is what slee pasted: I just can use perf to profile the cpu usage on B2G. Here are the steps. 1. download "profiling" branch of git://github.com/jld/B2G.git 2. remove out and gecko-output folder 3. modify .userconfig * export B2G_PROFILING=1 * If you specify GECKO_PATH and GECKO_OBJDIR in .userconfig, remember use absolute path. 4. apply the attached patch 5. disable elf-hack (I am not sure if it is necessary but I disable it.) 6. Build you project. Then you can use "./run-perf.sh record -a -g" to capture and "./run-perf.sh report" to get report. For the updated information, you can check bug 831611 - Get perf working on b2g.

Got a build with today's mozilla-central and the patch queue https://bugzilla.mozilla.org/attachment.cgi?id=739521 Unfortunately, upon joining a chat in 'gupshup', this consistently runs into the known Send__delete__ crash we have been struggling with, see attached stack; this is the same crash as in bug 864598.

I have a patch for this but I can't seem to be able to test this. When I go to the suggested WebRTC test site I never get a prompt that let's me allow the page access to my Unagi's camera.

Bas: go to the same page on your desktop computer, log in on both your computer and your unagi, and invite/call your unagi username from your computer session.

(In reply to Benoit Jacob [:bjacob] from comment #6) > Bas: go to the same page on your desktop computer, log in on both your > computer and your unagi, and invite/call your unagi username from your > computer session. Nothing happens.. my phone just sits there as far as I can tell. I'll try again.

(In reply to Bas Schouten (:bas.schouten) from comment #7) > (In reply to Benoit Jacob [:bjacob] from comment #6) > > Bas: go to the same page on your desktop computer, log in on both your > > computer and your unagi, and invite/call your unagi username from your > > computer session. > > Nothing happens.. my phone just sits there as far as I can tell. I'll try > again. I get the call, I accept, it opens, but it shows no video with either Local or Remote, and it doesn't seem to ask me if I want to enable my video.

Not sure what's going on; if you can get signaling:5 logs from the desktop that would help a lot (and I suspect be easier than getting them from the Unagi). You may want to use a debug build on desktop; more debugging info is available. If the issue is on the unagi side, you may need to ask the B2G guys how to get logs out. Interesting logs would include signaling:5,mediamanager:5,mtransport:5 Also: it helps when debugging to make sure the two devices (unagi and desktop) are on the same subnet.

So with the right patch queue I got things running. How I'm running into the cycle collector off-main-thread crash. I have seen bug 825110 but I'm not sure how to proceed. I still haven't been able to test this, in the meanwhile, I'll put my patch up for review here soon :)

This patch, together with the patch in bug 866521, should address this bug. I've been unable to test this due to the cycle collector crash though.

(In reply to Bas Schouten (:bas.schouten) from comment #10) > So with the right patch queue I got things running. How I'm running into the > cycle collector off-main-thread crash. I have seen bug 825110 but I'm not > sure how to proceed. I still haven't been able to test this, in the > meanwhile, I'll put my patch up for review here soon :) As talked on IRC, please update the patch to newer version and try again. It should work. If you find any problem in this part again, just let me know. :)

After applying some more patches sent by Chiajung it's now crashing here: 0x414aebfa in nsMainThreadPtrHolder (this=0x4badd0b0, ptr=0x4553e818, strict=<value optimized out>) at ../../dist/include/nsProxyRelease.h:116 116 MOZ_ASSERT(!mStrict || NS_IsMainThread()); (gdb) bt #0 0x414aebfa in nsMainThreadPtrHolder (this=0x4badd0b0, ptr=0x4553e818, strict=<value optimized out>) at ../../dist/include/nsProxyRelease.h:116 #1 0x414aecfa in GetPreviewStreamTask (this=0x4865a920, aSize=<value optimized out>, onSuccess=0x4553e818, onError=0x4553e824) at /home/bas/Dev/mozilla-central/dom/camera/CameraControlImpl.h:224 #2 mozilla::CameraControlImpl::GetPreviewStream (this=0x4865a920, aSize=<value optimized out>, onSuccess=0x4553e818, onError=0x4553e824) at /home/bas/Dev/mozilla-central/dom/camera/CameraControlImpl.cpp:333 #3 0x412070fa in mozilla::MediaEngineWebRTCVideoSource::Start (this=0x4553e800, aStream=0x4c3a04a0, aID=1) at /home/bas/Dev/mozilla-central/content/media/webrtc/MediaEngineWebRTCVideo.cpp:330 #4 0x4132adce in mozilla::MediaOperationRunnable::Run (this=0x4a5aa8a0) at /home/bas/Dev/mozilla-central/dom/media/MediaManager.h:296 #5 0x41bd3772 in nsThread::ProcessNextEvent (this=0x4b5d3b70, mayWait=<value optimized out>, result=0x4c1ffe87) at /home/bas/Dev/mozilla-central/xpcom/threads/nsThread.cpp:627 #6 0x41b9a088 in NS_ProcessNextEvent (thread=0x6a, mayWait=true) at /home/bas/Dev/mozilla-central/objdir-gonk/xpcom/build/nsThreadUtils.cpp:238 #7 0x41bd3dc6 in nsThread::ThreadFunc (arg=<value optimized out>) at /home/bas/Dev/mozilla-central/xpcom/threads/nsThread.cpp:265 #8 0x4086f2ea in _pt_root (arg=<value optimized out>) at /home/bas/Dev/mozilla-central/nsprpub/pr/src/pthreads/ptthread.c:204 #9 0x400cce18 in __thread_entry (func=0x4086f249 <_pt_root>, arg=0x4ba6b680, tls=<value optimized out>) at bionic/libc/bionic/pthread.c:217

Hmm...it seems like I have to check which thread Camera API expect its function to be called...I will update Bug 825110 as soon as I checked that.

It seems Camera callback are hold by nsMainThreadPtrHolder<T> now, which means I have to post all function call of camera to MainThread...I don't know when these are changed to such state, but I will fix my part later.

Comment on attachment 742872 [details] [diff] [review] Forward the surface descriptor for GrallocPlanarYCbCrImage Review of attachment 742872 [details] [diff] [review]: ----------------------------------------------------------------- Oh hey, I didn't suspect it would be so simple.

One question here: PlanarYCbCrImage should means a I420(Planar YUV) format image but the GraphicBuffer allocated in: http://mxr.mozilla.org/mozilla-central/source/gfx/layers/GrallocImages.cpp#43 is YV12 format, which is PlanarYVU format. I think it is easy to fix by swap UV channel pointers before memcpy.

And this cause crash here: http://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/TextureHostOGL.cpp#599 Since YV12 format is 0x32315659. You should add this (either the literal or include the correct header) into the list I think. If this patch is going to merge and close, I can file another bug for enable this feature for WebRTC.

Flagging Bas so he can't forget about these two comments ;-)

(In reply to Chiajung Hung [:chiajung] from comment #18) > And this cause crash here: > http://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/ > TextureHostOGL.cpp#599 > > Since YV12 format is 0x32315659. You should add this (either the literal or > include the correct header) into the list I think. If this patch is going to > merge and close, I can file another bug for enable this feature for WebRTC. See https://bugzilla.mozilla.org/show_bug.cgi?id=864017#c11. This is the reason you need the patch in bug 866521.

(In reply to Chiajung Hung [:chiajung] from comment #17) > One question here: > > PlanarYCbCrImage should means a I420(Planar YUV) format image but the > GraphicBuffer allocated in: > http://mxr.mozilla.org/mozilla-central/source/gfx/layers/GrallocImages.cpp#43 > > is YV12 format, which is PlanarYVU format. I think it is easy to fix by swap > UV channel pointers before memcpy. This code didn't change with the refactor. Am I correct to understand this was already broken?

(In reply to Chiajung Hung [:chiajung] from comment #17) > PlanarYCbCrImage should means a I420(Planar YUV) format image but the > GraphicBuffer allocated in: > http://mxr.mozilla.org/mozilla-central/source/gfx/layers/GrallocImages.cpp#43 > > is YV12 format, which is PlanarYVU format. I think it is easy to fix by swap > UV channel pointers before memcpy. Um, YV12 has the planes in the order Y', Cb, Cr. It is generally the format output by all of our software codecs. That should really be the same order used by PlanarYCbCrImage. There is quite a bit of confusion over what the "U" and "V" planes of "YUV" actually correspond to (which is why we label things YCbCr in our own code).

(In reply to Bas Schouten (:bas.schouten) from comment #11) > Created attachment 742872 [details] [diff] [review] > Forward the surface descriptor for GrallocPlanarYCbCrImage > > This patch, together with the patch in bug 866521, should address this bug. > I've been unable to test this due to the cycle collector crash though. Ok, I see. I did not notice that one.

(In reply to Timothy B. Terriberry (:derf) from comment #22) > (In reply to Chiajung Hung [:chiajung] from comment #17) > > PlanarYCbCrImage should means a I420(Planar YUV) format image but the > > GraphicBuffer allocated in: > > http://mxr.mozilla.org/mozilla-central/source/gfx/layers/GrallocImages.cpp#43 > > > > is YV12 format, which is PlanarYVU format. I think it is easy to fix by swap > > UV channel pointers before memcpy. > > Um, YV12 has the planes in the order Y', Cb, Cr. It is generally the format > output by all of our software codecs. That should really be the same order > used by PlanarYCbCrImage. > > There is quite a bit of confusion over what the "U" and "V" planes of "YUV" > actually correspond to (which is why we label things YCbCr in our own code). The naming is confusing for me, too. According to http://www.fourcc.org/yuv.php You can search for I420, and you will find YV12 0x32315659 12 8 bit Y plane followed by 8 bit 2x2 subsampled V and U planes. I420 0x30323449 12 8 bit Y plane followed by 8 bit 2x2 subsampled U and V planes. That's why I think the UV plane should be swapped.

(In reply to Bas Schouten (:bas.schouten) from comment #21) > (In reply to Chiajung Hung [:chiajung] from comment #17) > > One question here: > > > > PlanarYCbCrImage should means a I420(Planar YUV) format image but the > > GraphicBuffer allocated in: > > http://mxr.mozilla.org/mozilla-central/source/gfx/layers/GrallocImages.cpp#43 > > > > is YV12 format, which is PlanarYVU format. I think it is easy to fix by swap > > UV channel pointers before memcpy. > > This code didn't change with the refactor. Am I correct to understand this > was already broken? As comment 26 says, I think YV12 is different from I420 from definition.