848627 - [camera] Camera FPS of recorded video and the preview is really low ~18fps

Status: VERIFIED FIXED

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

Description

[Inder]

I have been investigating why the fps of the recorded videos and preview are really low (~18fps) on b2g and using the same hardware on android it's around 29 fps.
After working with camera team it turns out that the dequeuebuffer() in GonkNativeWindow.cpp is taking long time on b2g (> 50ms). I instrumented the code and confirm that most of the time it's > 50ms which causing the low FPS.

We need to optimize this function to get better FPS.

Comment 1

[Inder]

Attachment: DequeueBuffer performance on FFOS

Comment 2

[Inder]

Attachment: DequeueBuffer performance on Android

You can see how it takes so much more time on FFOS. On Android it max goes to the order of 15ms where as on FFOS it takes up to .7 seconds!
The numbers were collected on QRD.

Comment 3

[Wayne Chang [:wchang]]

Hi Ben,
Please help with this one.
thanks.

Comment 4

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

dequeueBuffer() takes as long as it does because it often blocks waiting for a buffer to get returned from the viewfinder MediaStream.  (I'm not certain why this takes so long, but it may be because the MediaStream likes to keep extra frames in its queue, depleting the pipeline.)

I've CCed sotaro, since he was working on bug 844248 to address this issue.

Comment 5

[Andreas Gal :gal]

Can we unhide this bug?

Comment 6

[Sotaro Ikeda [:sotaro]]

(In reply to Inder from comment #0)
> I have been investigating why the fps of the recorded videos and preview are
> really low (~18fps) on b2g and using the same hardware on android it's
> around 29 fps.
> After working with camera team it turns out that the dequeuebuffer() in
> GonkNativeWindow.cpp is taking long time on b2g (> 50ms). I instrumented the
> code and confirm that most of the time it's > 50ms which causing the low FPS.

The problem happens because GonkNativeWindow throws gralloc buffers out of the class and the buffers are rendered in Compositor. Camera hal tries to dequeue buffers from GonkNativeWindow until a number of UNDEQUEUED_BUFFERS becomes MIN_UNDEQUEUED_BUFFERS(2). But more than 2 buffers are out of GonkNativeWindow. Then Camera hal always needs to wait buffer return.

There are 2 ways to solve this problem. Both ways are very risky and need more gralloc buffers.
[1] Increment MIN_UNDEQUEUED_BUFFERS from 2 to some value. Ensure the max number of gralloc buffers between GonkNativeWindow and Compositor. Modify camera hal to accept MIN_UNDEQUEUED_BUFFERS more than 2.

[2] Copy preview frames for rendering and GonkNativeWindow run in async mode.  The copied frames are throwed out of GonkNativeWindow for rendering. Original frames stay within GonkNativeWindow.

[1] needs nodification to camera hal and MediaStreamGraph. And more GraphicBuffers
Camera hal of unagi does not work in MIN_UNDEQUEUED_BUFFERS more than 2.

[2] needs a cpu time for copy video frames. And more GraphicBuffers. But it does not scale to high resolution preview.

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: Crash log of unagi when UNDEQUEUED_BUFFERS is 3

When I change the UNDEQUEUED_BUFFERS from 2 to 3 on unagi. The camera hal cause abort with the log during starting preview.

Comment 8

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

Inder, what do you think about ^^^?

Comment 9

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #6)
> (In reply to Inder from comment #0)
//snip
> There are 2 ways to solve this problem. Both ways are very risky and need
> more gralloc buffers.
> [1] Increment MIN_UNDEQUEUED_BUFFERS from 2 to some value. Ensure the max
> number of gralloc buffers between GonkNativeWindow and Compositor. Modify
> camera hal to accept MIN_UNDEQUEUED_BUFFERS more than 2.
> 
> [2] Copy preview frames for rendering and GonkNativeWindow run in async
> mode.  The copied frames are throwed out of GonkNativeWindow for rendering.
> Original frames stay within GonkNativeWindow.

In Bug 844248, I created attachment 724585 [details] [diff] [review] as temporary patch of [2]. Copying of video frame(480*320 in camera mode) takes 10ms-25ms(huge!). And after when I took a picture, camera app abort because of out of memory(gralloc buffer) during restarting preview.

Then [1] is a correct way to fix the problem.

Comment 10

[Inder]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #8)
> Inder, what do you think about ^^^?

Sotaro, I am still not clear about why Android works well with MIN_UNDEQUEUED_BUFFERS=2 whereas on FFOS we need >2.

Comment 11

[Ben Tian (inactive)]

Can we bypass MediaStreamGraph to show camera preview on display directly? I wonder the necessity of MediaStreamGraph for camera preview and didn't get why we need it to render camera preview to video tag.

Comment 12

[Sotaro Ikeda [:sotaro]]

(In reply to Inder from comment #10)
> (In reply to Michael Vines [:m1] [:evilmachines] from comment #8)
> > Inder, what do you think about ^^^?
> 
> Sotaro, I am still not clear about why Android works well with
> MIN_UNDEQUEUED_BUFFERS=2 whereas on FFOS we need >2.

<SurfaceTexture in android case>
Buffers are managed by following states in SurfaceTexture
 - FREE, DEQUEUED, QUEUED
And when rendering camera preview, SurfaceTexture works in async mode.
In the async mode, only one buffer is set to QUEUED state.
And currentTexture's buffer is set to FREE state.
The above two buffers are need to be undequeued. 
So, MIN_UNDEQUEUED_BUFFERS=2 works.

<GonkNativeWindow in Firefox OS case>
Buffers are managed by following states in SurfaceTexture
 - FREE, DEQUEUED, QUEUED, RENDERING

Buffers are set to RENDERING state when buffers are sent to the compositor.
The number of buffers in RENDERING state exceed more than two.
MediaStreamGraph is one big area of buffering buffers.
Then MIN_UNDEQUEUED_BUFFERS=2 do not works.

Comment 13

[Sotaro Ikeda [:sotaro]]

Also in android, SurfaceMediaSource(ISurfaceTexture derived class) sets MIN_UNDEQUEUED_BUFFERS=4.

http://androidxref.com/4.0.4/xref/frameworks/base/include/media/stagefright/SurfaceMediaSource.h#37

Comment 14

[Sotaro Ikeda [:sotaro]]

(In reply to Ben Tian [:bentian] from comment #11)
> Can we bypass MediaStreamGraph to show camera preview on display directly? I
> wonder the necessity of MediaStreamGraph for camera preview and didn't get
> why we need it to render camera preview to video tag.

I do not understand well about "show camera preview on display directly?". Can you explain about it? Camera preview needs to be layouted and buffers of the preview frame needs to be sent to compositor via ImageContainerChild. Followings is a diagram of current configuration.

https://github.com/sotaroikeda/firefox-diagrams/blob/master/dom/dom_camera_FirefoxOS_1_1.pdf?raw=true

Comment 15

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

(In reply to Sotaro Ikeda [:sotaro] from comment #14)
> 
> I do not understand well about "show camera preview on display directly?".

Instead of setting the mozSrcObject of a <video> tag to a MediaStream, could we just have an <img> tag and somehow push frames to it?

Comment 16

[Sotaro Ikeda [:sotaro]]

camera needs to provide MediaStream also for other use like Bug 842243. I am thinking to create custom MediaStream for camera preview to solve buffering and delay problem.

Comment 17

[Ben Tian (inactive)]

(In reply to Sotaro Ikeda [:sotaro] from comment #14)
> I do not understand well about "show camera preview on display directly?".
> Can you explain about it? Camera preview needs to be layouted and buffers of
> the preview frame needs to be sent to compositor via ImageContainerChild.
> Followings is a diagram of current configuration.
> 
> https://github.com/sotaroikeda/firefox-diagrams/blob/master/dom/
> dom_camera_FirefoxOS_1_1.pdf?raw=true

I mean 'creating a shortcut from camera frame to ImageContainerChild.' My idea is to omit the whole MediaStreamGraph if camera preview doesn't require it.

Thanks for the camera diagrams. They help a lot.

(In reply to Sotaro Ikeda [:sotaro] from comment #16)
> camera needs to provide MediaStream also for other use like Bug 842243. I am
> thinking to create custom MediaStream for camera preview to solve buffering
> and delay problem.

This is another approach. I wonder whether 1) to omit MediaStreamGraph or 2) to customize it is less risky.

Comment 18

[Sotaro Ikeda [:sotaro]]

In Bug 844248, I created a temporary patch for b2g18 attachment 726242 [details] [diff] [review] . 

By applying the patch, fps of preview becomes better and camera/video mode change becomes quicker.

Comment 19

[Joe Cheng [:jcheng] (please needinfo)]

:sotaro, since you are working on this, mind taking this bug?

Comment 20

[Sotaro Ikeda [:sotaro]]

(In reply to Joe Cheng [:jcheng] from comment #19)
> :sotaro, since you are working on this, mind taking this bug?

jcheng, which problem are you going to solve?

From my point of view, the problem is divided to two problems.
[1] latency and buffering at MediaStreamGraph
[2] MIN_UNDEQUEUED_BUFFERS=2 is not enough for FirefoxOS 

I am going to fix [1] at Bug 844248. I got positive feedback about attachment 726207 [details] [diff] [review] from roc. I am going to fix [1] soon.

[2] needs partner's fix. I do not worikng about [2].

Comment 21

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #20)
> (In reply to Joe Cheng [:jcheng] from comment #19)
> > :sotaro, since you are working on this, mind taking this bug?
> 
> jcheng, which problem are you going to solve?
> 
> From my point of view, the problem is divided to two problems.
> [1] latency and buffering at MediaStreamGraph
> [2] MIN_UNDEQUEUED_BUFFERS=2 is not enough for FirefoxOS 
> 
> I am going to fix [1] at Bug 844248. I got positive feedback about
> attachment 726207 [details] [diff] [review] from roc. I am going to fix [1]
> soon.

I am working for Bug 844248. So I do not mind you take this bug.

Comment 22

[Sotaro Ikeda [:sotaro]]

(In reply to Joe Cheng [:jcheng] from comment #19)
> :sotaro, since you are working on this, mind taking this bug?

Sorry, I misunderstand about it. I am going to take the bug.

Comment 23

[Sotaro Ikeda [:sotaro]]

> > 
> > From my point of view, the problem is divided to two problems.
> > [1] latency and buffering at MediaStreamGraph
> > [2] MIN_UNDEQUEUED_BUFFERS=2 is not enough for FirefoxOS 
> > 

Inder, [1] is going to be fixed at Bug 844248 soon. This fix improve preview and recording quality very much.

Can you re-evaluate the quality again after the fix? It is enough for v1.01, I think.

Comment 24

[Inder]

Sure, let me apply the patch and see how much improvement we get.

Comment 25

[Inder]

Sotaro: with the change in bug 844248 I am seeing fps as ~29.5 which is same as what we get on Android! So, it definitely fixes this issue. Lets get the fix landed soon.

Comment 26

[Inder]

Attachment: DequeueBuffer performance with Sotaro patch on FFOS

Just to make sure that the patch in bug 844248 is helping with the FPS performance. I measured the DequeueBuffer timing on FFOS with Sotaro's patch in bug 844248. There is tremendous improvement in the timing. Though there are instances when it does take > 20ms. But those instances are very few and doesn't affect the FPS performance which is around 29.5.

Comment 27

[Wayne Chang [:wchang]]

Inder, are we concerned about [2] from comment 20? 
Or per your comment 26 we're OK to close off this bug as a DUP to bug 844248 and perhaps track [2] elsewhere as it shouldn't block the release? (correct me if I have the wrong idea).

Comment 28

[Inder]

We are able to get ~29.5 FPS by just fixing bug 844248 so I don't think we need make [2] change in comment 20. I am marking this bug resolved.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Calling this fixed by bug 844248 per comment 28.

Comment 30

[mdaniloff]

Verified Fixed on Unagi. With Show Frames per second setting enabled, user created video indicated 29-35 FPS.
Unagi Build ID: 20130322070203
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/4931ec89ebbe
Gaia: 85fd164691bb852f1cfaf82405df4380629ced6e