1039937 - [dolphin] Camera performance is very bad (camera launch, take pic or video very slow)

Status: VERIFIED FIXED

(Core :: Graphics, defect, P1)

Description

[Peipei Cheng (needinfo if you need my action)]

Attachment: camera.log

This is a regression from build 20140715160201.

Steps to reproduce
---------------------------------------------------------------------------
1. Flash build 20140716160201 (https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-b2g30_v1_4-dolphin/2014/07/2014-07-16-16-02-01/)
2. Launch Camera
   --> Launch time is over 10s
3. Take a picture
   --> It took over 10s for user to take a picture
4. Switch to Video mode
   --> Switch time is over 5s
5. Take a video
   --> It took over 10s to start taking video

Please see attached adb log. Test starts: 07-17 13:51:24.013


Test build:
Gaia      e0a732cf172d125eadfad59f0faf8f73b62012b8
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/a429f0b6206d
BuildID   20140716160201
Version   30.0
ro.build.version.incremental=282
ro.build.date=Thu Jul 10 10:27:07 CST 2014

Comment 1

[Peipei Cheng (needinfo if you need my action)]

I also recorded a video here: https://mega.co.nz/#!1oVFRJDB!Fx3vWSVH7UlRbSWIbGVBrUNAoNZWp7D5ltloS5TCVZA

Comment 2

[Mike Lien[:mlien]]

fallback gecko to https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/932c37978d37 will be fine

Comment 3

[Xiaohong.Fan]

sprd bug 334521 is similar with this bug.

Comment 4

[Peipei Cheng (needinfo if you need my action)]

regression caused by the change of bug 1035755

Comment 5

[Sotaro Ikeda [:sotaro]]

Camera performance is very bad even before bug 1035755.

For dolphin device, camera task seem to try to do too much things. But around camera dom, there seems very little thing that we can do. Around graphics, there are 2 improvement in gecko. They might improve the performance. But they are very big changes. It could cause regressions. Therefore it is not a good ides to uplift to v1.4 except dolphin has a dedicated branch like "v1.3t".

The two Bug 985772 and Bug 959089.

Bug 985772 reduces the TextureClient re-creation. Until v1.4, TextureClient is re-created for each video frame rendering. It was one time use. During TextureClient destruction, some IPC is used one is sync IPC. It could slow down the ImageBridge performance.

Bug 959089 improves gralloc buffer allocation. Until v1.4 gralloc buffer is allocated on Compositor thread in b2g process. This bug change the gralloc buffer allocation to a dedicated thread in b2g process.

Comment 6

[Peipei Cheng (needinfo if you need my action)]

but before bug 1035755, the launch time and the response time are under 5s. But after this bug fix, the response time could be longer than 20s sometimes which is not acceptable.

Comment 7

[Sotaro Ikeda [:sotaro]]

(In reply to pcheng from comment #6)
> but before bug 1035755, the launch time and the response time are under 5s.
> But after this bug fix, the response time could be longer than 20s sometimes
> which is not acceptable.

You are talking just about the symptom. Did you analyze why bug 1035755 regress the performance?

I did not analyze about it, but I assume Bug 985772 is related. Before Bug 985772 fix, gecko uses TextureClient as one time use and it adds extra task to ImageBridge. Before but 1035755, the extra task is deferred, but since bug 1035755 fix, the extra task is not be deferred. It might make a difference. It is just my assumption.

Comment 8

[Jason Smith [:jsmith]]

Does this happen on Flame?

Comment 9

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Jason Smith [:jsmith] from comment #8)
> Does this happen on Flame?

Issue is NOT observed on Flame on the same build ID 20140715160201.

It takes about 3 seconds to cold launch camera, 2 seconds to complete taking a picture, and unable to record a video in 273MB mem which is expected and has been bugged somewhere.

In 512mb mem Flame everything works as expected without issues.

Tested on:
Device: Flame
Build ID: 20140715160201
Gaia: 393d72937727ad20e82b2ff7b13e3d7ff077a9f0
Gecko: 932c37978d37
Version: 30.0 (1.4)
Firmware Version: v122

Comment 10

[Sotaro Ikeda [:sotaro]]

Attachment: patch for b2g v1.4 - Remove SurfaceDescriptor around GonkNativeWindow

The patch includes Bug 985772, Bug 986933 and Bug 1014360. The patch fixes the problem for me on v1.4 dolphin. This patch might improve other dolphin's camera performance bugs.

Comment 11

[Sotaro Ikeda [:sotaro]]

attachment 8459717 [details] [diff] [review] provides enough improvement for dolphin even when the patch has a regression risk.

Comment 12

[Ivan Tsay (:ITsay)]

Hi William,

Could you help to locally test the patch provided by Sotaro im comment 11 and see if there is any regression? I think this patch is very valuable for resolving the dolphin camera performance issue. Since Sotaro metinoed about the risk, it is better we can test it before land it in v1.4.

Comment 13

[William Hsu [:whsu]]

(In reply to Ivan Tsay (:ITsay) from comment #12)
> Hi William,
> 
> Could you help to locally test the patch provided by Sotaro im comment 11
> and see if there is any regression? I think this patch is very valuable for
> resolving the dolphin camera performance issue. Since Sotaro metinoed about
> the risk, it is better we can test it before land it in v1.4.

Sure! I will verify it on local build.
Keep needinfo "whsu"
ETA: 7/24 (UTC +8)

Comment 14

[Andreas Gal :gal]

Why is this not landed on trunk yet? 2.0 is waiting for this as well. Please land and worry about uplifts second.

Comment 15

[Sotaro Ikeda [:sotaro]]

(In reply to Andreas Gal :gal from comment #14)
> Why is this not landed on trunk yet? 2.0 is waiting for this as well. Please
> land and worry about uplifts second.

The patch is actually an uplift of Bug 985772. Correspondence things are already on trunk and b2g-v2.0. The patch can not be simply uplifted to v1.4, because there are some differences. Therefore, I created attachment 8459717 [details] [diff] [review] for b2g-v1.4.

Comment 16

[Sotaro Ikeda [:sotaro]]

attachment 8459717 [details] [diff] [review] is moved to Bug 985772, because it is just uplift of Bug 985772 fix.

Comment 17

[Rachelle Yang [:ryang][[e-mail]]]

Hi William, ay update of the testing result of the patch? Thanks!

Comment 18

[Vincent Liu[:vliu]]

Attachment: profiling_for_taking_picture.ods

I did some profiling for camera taking picture case on Dolphin, Flame and Buri. I hope it can help to narrow down the performance issue.

1. For measuring time of auto focus, Dolphin runs about 8 times than other devices. Please see column B for more detailed. 

2. For measuring time of taking picture, Dolphin sometimes takes above 2 seconds when the first launch after reboot.

Comment 19

[William Hsu [:whsu]]

Thanks Sotaro and Vincent!
I verified the patch on my local build. (rollup patch for b2g-v1.4 of bug 985772)
It improve the camera performance.
Please uplift this patch to v1.4.

* Build information:
 - Gaia      5db3d93d68c30c1cf24499b3cda3fd881d207ea6
 - Gecko     9e663197e4e0622f6576e0dd32351f6ec0027405
 - BuildID   20140724150446
 - Version   30.0

Comment 20

[Kevin Simons [:kevsim] (Telenor)]

Sotaro - is it possible to have this patch not remove GraphicBufferLocked class? It's ok not to use it, but can we please keep the class around? It is used in our video call implementation currently.

Comment 21

[Sotaro Ikeda [:sotaro]]

(In reply to Kevin Simons [:kevsim] (Telenor) from comment #20)
> Sotaro - is it possible to have this patch not remove GraphicBufferLocked
> class? It's ok not to use it, but can we please keep the class around? It is
> used in our video call implementation currently.

GraphicBufferLocked has a dependency to GrallocImage and GrallocTextureClientOGL classes. Therefore, just keep the GraphicBufferLocked definition seems not work correctly. Sorry, it seems not possible to keep GraphicBufferLocked after uplifting the patch.

Comment 22

[Sotaro Ikeda [:sotaro]]

The patch was uplifted to b2g-v1.4.

Comment 23

[William Hsu [:whsu]]

Hi, Peipei,

I would like to mark this bug as duplicate of bug 985772.
Do you agree?
Thanks.

Comment 24

[Peipei Cheng (needinfo if you need my action)]

(In reply to William Hsu [:whsu] from comment #23)
> Hi, Peipei,
> 
> I would like to mark this bug as duplicate of bug 985772.
> Do you agree?
> Thanks.

I think it's ok. keep ni on me to check when new build comes out.

Comment 25

[Peipei Cheng (needinfo if you need my action)]

I still see the same behavior on the latest dolphin build. So the fix for bug 985772 does not fix this problem.
Reopen it.

Gaia      eb3b185325901d4c04e2d43eb58d90835213bea9
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/8c8883bb5797
BuildID   20140728160206
Version   30.0
ro.build.version.incremental=314
ro.build.date=Tue Jul 22 20:54:56 CST 2014

Comment 26

[Peipei Cheng (needinfo if you need my action)]

(In reply to Peipei Cheng from comment #25)
> I still see the same behavior on the latest dolphin build. So the fix for
> bug 985772 does not fix this problem.
> Reopen it.
> 
> Gaia      eb3b185325901d4c04e2d43eb58d90835213bea9
> Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/8c8883bb5797
> BuildID   20140728160206
> Version   30.0
> ro.build.version.incremental=314
> ro.build.date=Tue Jul 22 20:54:56 CST 2014

seems the pvt has problem. the gecko commit is on July 13th. depend on 1045453

Comment 27

[Peipei Cheng (needinfo if you need my action)]

(In reply to Peipei Cheng from comment #26)
> (In reply to Peipei Cheng from comment #25)
> > I still see the same behavior on the latest dolphin build. So the fix for
> > bug 985772 does not fix this problem.
> > Reopen it.
> > 
> > Gaia      eb3b185325901d4c04e2d43eb58d90835213bea9
> > Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/8c8883bb5797
> > BuildID   20140728160206
> > Version   30.0
> > ro.build.version.incremental=314
> > ro.build.date=Tue Jul 22 20:54:56 CST 2014
> 
> seems the pvt has problem. the gecko commit is on July 13th. depend on
> 1045453

Release team confirmed that it's not build problem.

Can anyone take a look at the adb log attached? I saw such errors:

07-17 13:52:10.423 E/        (  104): Failed to open file '/system/media/audio/ui/camera_click.ogg'. (No such file or directory)
07-17 13:52:10.423 E/MediaPlayer(  104): error (1, -2147483648)
07-17 13:52:10.423 I/AwesomePlayer(  104): setDataSource_l(URL suppressed)
07-17 13:52:10.433 E/        (  104): Failed to open file '/system/media/audio/ui/VideoRecord.ogg'. (No such file or directory)
07-17 13:52:10.433 E/MediaPlayer(  104): error (1, -2147483648)

Comment 28

[Wayne Chang [:wchang]]

Looks like just the sound clip files missing?

Vincent?

Comment 29

[Peipei Cheng (needinfo if you need my action)]

An update here:
Partner's latest base image 20140725 does not has this problem.
But If I flash gecko and gaia on top of this base image, this performance problem appears again.

Comment 30

[Peipei Cheng (needinfo if you need my action)]

Finally find the reason, 985772 is backed out before pvt build become normal (https://bugzilla.mozilla.org/show_bug.cgi?id=985772#c60). So we never see how the fix works. 

Leave this bug open and ni on me to test again after the commit of bug 985772 uplifts.

Comment 31

[Brian Huang [:brianhuang][:brhuang]]

Peipei, bug 985772 is fixed. Could you check it again?

Comment 32

[Peipei Cheng (needinfo if you need my action)]

(In reply to Brian Huang [:brianhuang][:brhuang] from comment #31)
> Peipei, bug 985772 is fixed. Could you check it again?

I tested on latest pvt build. The fix for bug 985772 works.

Gaia      3feb37ee2ed2319c9e556728723a5517dc1663ea
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/2b5defe2d811
BuildID   20140731160213
Version   30.0
ro.build.version.incremental=320
ro.build.date=Fri Jul 25 13:40:20 CST 2014