994077 - [Camera][Hamachi][Dolphin] Long wait for initial preview stream

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Camera, defect, P1)

Description

[Wilson Page [:wilsonpage]]

I've noticed that Hamachi can sometimes take > 6sec to retrieve the camera preview stream. Wondering if there is anything that can be done on Gecko side to speed this up.

Comment 1

[Wilson Page [:wilsonpage]]

Attachment: video

Comment 2

[Mike Habicher [:mikeh] (high bugzilla latency)]

The last time I measured this, ~2 seconds of the start-up time can be attributed to starting the driver and opening the camera hardware, with about another 100ms of overhead across everything else in Gecko.

See bug 817051, bug 821841, bug 833114, and bug 835367.

Can you take some measurements? Specifically, how long it takes getCamera() to return, and the time between getCamera() returning and its onSuccess callback getting invoked.

Comment 3

[Wilson Page [:wilsonpage]]

On my Hamachi I record 3 seconds between request and callback. If this is unavoidable, we can just close this issue now.

Comment 4

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Wilson Page [:wilsonpage] from comment #3)
>
> On my Hamachi I record 3 seconds between request and callback. If this is
> unavoidable, we can just close this issue now.

There are a lot of historical bugs related to Camera start-up time. Take a look at bug 914916 comment 4 for some detailed information, and also bug 915083.

Regardless, taking ~6 seconds to launch is far too long.

Comment 5

[Hema Koka [:hema]]

6 secs!...not acceptable, lets investigate what is going on

Comment 6

[Wilson Page [:wilsonpage]]

I'm currently working on trimming JS startup time to an absolute minimum to improve the 'critical-path' (time to viewfinder). On the Hamachi the majority of this time is waiting for Gecko to give us the camera object.

Comment 7

[Hema Koka [:hema]]

I am leaving this assigned to you since you are currently profiling and figure out whats going on?

Comment 9

[Wilson Page [:wilsonpage]]

Attachment: pull-request (master)

Comment 10

[Wilson Page [:wilsonpage]]

This is a big patch. Probably risky to uplift this to 1.4.

Comment 12

[Diego Marcos [:dmarcos]]

You can land after addressing comments on github.

Comment 13

[Diego Marcos [:dmarcos]]

It's important tho check how these changes affect the numbers in perf-o-matic:

https://datazilla.mozilla.org/b2g/?branch=master&device=hamachi&range=7&test=cold_load_time&app_list=camera&app=camera&gaia_rev=553da99ab09b6b89&gecko_rev=5b6e82e7bbbf&plot=avg

Comment 14

[Tony Chung [:tchung]]

(In reply to Wilson Page [:wilsonpage] from comment #10)
> This is a big patch. Probably risky to uplift this to 1.4.

Hema, based on what Wilson is saying, do we really think this should still be a 1.4+ blocker?   I do agree that the launch time is noticably slow (tested myself), but i want to assess the risk of landing here.  Please comment, thanks.

Comment 15

[Hema Koka [:hema]]

As discussed earlier, this patch is big and risky to land into 1.4. Can you check to see if there is a low-risk change from your work that will help us on the perf front that we can comfortably take in for 1.4 release?

Thanks for this work including the tests that you have added/updated in this patch.

Once the changes are reviewed (also get djf to do one more review since it is a big patch - 2 pairs of eyes will be good) with necessary tests, we can land them into master for 2.0 release. It will give us some time to bake and fix any issues for next release.

Back into 1.4? bucket for reconsideration after I hear back from you on low-risk fixes if any for 1.4

Thanks
Hema

Thanks
Hema

Comment 16

[Jason Smith [:jsmith]]

The one problem I think we have here is the fact that the viewfinder is taking too long to load could be a blocker against our performance certification criteria.

Question:

Is this issue device independent or specific to Buri?

Comment 17

[Diego Marcos [:dmarcos]]

What's the performance criteria?

Comment 18

[Jason Smith [:jsmith]]

(In reply to Diego Marcos [:dmarcos] from comment #17)
> What's the performance criteria?

Mike might know that answer.

Comment 19

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

I don't think we have a hard internal acceptance number defined for this yet. Our numbers are going to be pretty coarse-grained for 1.4 given timeframe and probably won't include many app-internal operations.

That said, if this is considered to be part of camera cold-launch, our rough guideline for *any* cold launch to user-ready is ~1.25s max. 

Camera's one of them that I know we'd have to be looser on (I don't think Android's even near that target) but 6s is way, way too high IMO. For comparison, we already have bug 995303 against 1.3T/Tarako at 2.9 secs. I'd expect 1.4/Hamachi to be faster.

But all said, Jason's referring to external criteria via IOT, partner, etc. I haven't heard much about camera performance stats there, but I expect this to get called out if we leave it as-is.

Comment 20

[Diego Marcos [:dmarcos]]

Do you have a consistent way to reproduce those 6s on startup time that you claim? I'm collecting numbers but I haven't been able to see such a long startup time on my hamachi.

Comment 21

[:Eli Perelman]

Geo is correct, the start-up time goal for all apps is 1.25 seconds [1]. If that is not possible, then some UI needs to be shown that gives the user indication that something is happening.

[1] https://developer.mozilla.org/en-US/Apps/Build/Performance/Firefox_OS_app_responsiveness_guidelines

Comment 22

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Eli Perelman from comment #21)
>
> Geo is correct, the start-up time goal for all apps is 1.25 seconds [1]. If
> that is not possible, then some UI needs to be shown that gives the user
> indication that something is happening.

Then we'll need something like that for Hamachi and Buri, as the camera hardware on these devices takes ~2s to complete the single call to connect().

Comment 23

[Jason Smith [:jsmith]]

I think this needs more discussion before we conclude moving this to a future release.

Comment 24

[Jason Smith [:jsmith]]

Discussed this with rel man - here's what we should do for 1.4:

For 1.4, we will not take the same patch we are taking for master. However, we'll need to come up with something more low risk to mitigate the fact that there's a long startup time for the viewfinder on devices like Buri. The suggestion that comment 21 seems like the right direction - let's implementation a progress indicator when the viewfinder hasn't loaded. To simplify tracking, I'm going to open a new bug for this & move this bug out to 2.0.

Comment 25

[Jason Smith [:jsmith]]

bug 999171 has been filed to track the 1.4 solution.

Comment 26

[Diego Marcos [:dmarcos]]

I've been playing today with my Hamachi and I cannot reproduce the 6s that wilson claims. The time from tap to first picture is around 3.5s (Around 2.5s waiting for the camera hardware to provide the preview). This startup time has remained unchanged since 1.2. We have to try to improve this but the bottleneck comes from the bottom of the stack: Camera Driver / Gonk. I don't know who has the expertise to work on this. Mike might point us to the right direction

Comment 27

[Jason Smith [:jsmith]]

(In reply to Diego Marcos [:dmarcos] from comment #26)
> I've been playing today with my Hamachi and I cannot reproduce the 6s that
> wilson claims. The time from tap to first picture is around 3.5s (Around
> 2.5s waiting for the camera hardware to provide the preview). This startup
> time has remained unchanged since 1.2. We have to try to improve this but
> the bottleneck comes from the bottom of the stack: Camera Driver / Gonk. I
> don't know who has the expertise to work on this. Mike might point us to the
> right direction

This doesn't always happen 100% of the time. Of the three times I tried, I reproduced this 2/3 times.

Comment 28

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #27)
> (In reply to Diego Marcos [:dmarcos] from comment #26)
> > I've been playing today with my Hamachi and I cannot reproduce the 6s that
> > wilson claims. The time from tap to first picture is around 3.5s (Around
> > 2.5s waiting for the camera hardware to provide the preview). This startup
> > time has remained unchanged since 1.2. We have to try to improve this but
> > the bottleneck comes from the bottom of the stack: Camera Driver / Gonk. I
> > don't know who has the expertise to work on this. Mike might point us to the
> > right direction
> 
> This doesn't always happen 100% of the time. Of the three times I tried, I
> reproduced this 2/3 times.

I tested this btw by doing a cold launch of the camera app with 1.3 & 1.4 next to each other.

Comment 29

[Diego Marcos [:dmarcos]]

To address the long waits for the camera preview on hamachi we need a loading bar / icon to keep the user informed about what's going on.

Comment 30

[Jason Smith [:jsmith]]

(In reply to Diego Marcos [:dmarcos] from comment #29)
> To address the long waits for the camera preview on hamachi we need a
> loading bar / icon to keep the user informed about what's going on.

Note - I already needinfo UX over in bug 999171 about this.

Comment 31

[Wilson Page [:wilsonpage]]

dmarcos: I guess the 6sec startup time could be influenced by many platform variables. Justin said he noticed improvements over time. This makes this especially hard to work on as we can't determine where improvements and regressions are coming from.

If we really want the gains this patch brings in 1.4, perhaps it would be more efficient to polish and test the patch. It does have test coverage in areas that master doesn't currently have. I'd be comfortable landing if we got more eyes on this.

Comment 32

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Diego Marcos [:dmarcos] from comment #26)
>
> This startup
> time has remained unchanged since 1.2. We have to try to improve this but
> the bottleneck comes from the bottom of the stack: Camera Driver / Gonk. I
> don't know who has the expertise to work on this. Mike might point us to the
> right direction

This is a vendor issue. Once upon a time we did a lot of digging into where the device is spending its time during this 2 seconds, and it's mostly opaque to us. The areas where we do have visibility happen quickly (think 10 to 100ms)--comparable to other platforms.

Comment 33

[Amy Lee [:amylee] UX]

(In reply to Diego Marcos [:dmarcos] from comment #29)
> To address the long waits for the camera preview on hamachi we need a
> loading bar / icon to keep the user informed about what's going on.

Hi, 

I think it makes the most sense to use the spinner from our building blocks as the loading animation

http://buildingfirefoxos.com/building-blocks/progress-and-activity.html

Thanks!

Comment 34

[Tiffanie Shakespeare [:tif]]

Amy has it correct - we should show a spinner since we can't predict how long the delay will be.

Comment 35

[Wilson Page [:wilsonpage]]

Comment on attachment 8406224 [details] [review]
pull-request (master)

This needs a second review. If you would be so kind Gents... :)

Comment 36

[Diego Marcos [:dmarcos]]

With this patch applied the camera doesn't start anymore from the lock screen with password. When tapping on the camera button the screen goes black and camera doesn't start

Comment 37

[Diego Marcos [:dmarcos]]

I made a b2gperf --delay=10 --iterations=30 camera run for both master and the branch of this bug and I didn't see any regression. Below the numbers:

MASTER

2014-04-23 16:35:04,488 B2GPerfRunner INFO    | Results for camera, cold_load_time: median:1131, mean:1125, std: 24, max:1164, min:1025, all:1141,1083,1136,1138,1123,1112,1132,1155,1136,1118,1136,1131,1140,1143,1125,1113,1151,1127,1025,1142,1120,1164,1093,1124,1117,1136,1134,1116,1130,1134

PERF BRANCH

2014-04-23 16:43:37,460 B2GPerfRunner INFO    | Results for camera, cold_load_time: median:1128, mean:1136, std: 40, max:1298, min:1074, all:1153,1103,1134,1153,1132,1129,1117,1298,1107,1114,1126,1115,1138,1100,1133,1122,1113,1225,1139,1152,1154,1074,1139,1099,1123,1127,1117,1168,1158,1120

Comment 38

[Wilson Page [:wilsonpage]]

dmarcos: That's good news. I'll remove startup.js then.

Comment 39

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8406224 [details] [review]
pull-request (master)

Considering how large this patch is, it looks like it is in really good shape. I haven't pulled it down and run it, I simply read through the code. I have a few nits that need addressed, but one in particular was a 1-line change in the SMS app that I'm not sure if it was accidental/testing. Please check and see if that was intentional before landing. Conditional R+

Comment 40

[Diego Marcos [:dmarcos]]

Land this puppy

Comment 41

[Wilson Page [:wilsonpage]]

Attachment: pull-request (master)

Comment 42

[Diego Marcos [:dmarcos]]

Wilson, The unit tests failed on travis

Comment 43

[Justin D'Arcangelo [:justindarc]]

Fixed unit tests. Waiting for our buddy Travis to go green again!

Comment 44

[Diego Marcos [:dmarcos]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/59f8b3723e9fc0f7d9fe838540ce5c6a9d02b2b6

Comment 47

[Jason Smith [:jsmith]]

Tarako is not affected by this bug. This issue only applies to the new camera implementation that landed in 1.4.