[flatfish][camera] Camera preview performance is bad

UNCONFIRMED
Unassigned

Status

Firefox OS
Gaia::Camera
P4
normal
UNCONFIRMED
4 years ago
3 years ago

People

(Reporter: JustinLee, Unassigned, NeedInfo)

Tracking

(Blocks: 1 bug, {perf})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [c= p= s= u=flatfish] [TCP=performance][flatfish])

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Now camera preview performance is bad on flatfish platform. Kernel message shows below when opening camera preview.

s_fmt set width = 2560, height = 1920
[ 2167.181416] [VFE]buffer_setup, buffer count=10, size=7372800

It seems the preview size has been set to 2560*1920 as default. The setting (2560*1920) causes the preformance of camera preview is bad.
Do you have any suggestion for camera preview size?

Comment 1

4 years ago
Justin has actually a simple work-around on this issue in Gaia. Could you describe it here? Thanks.
(Reporter)

Updated

4 years ago
Blocks: 903304
(Reporter)

Comment 2

4 years ago
I modified geco/dom/camera/GonkCameraControl.cpp to set preview size to 640*480. The performance of camera preview got improvement by this modification. The below shows my modification.

---
 dom/camera/GonkCameraControl.cpp |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/dom/camera/GonkCameraControl.cpp b/dom/camera/GonkCameraControl.cpp
index 9e5a75a..c066a16 100644
--- a/dom/camera/GonkCameraControl.cpp
+++ b/dom/camera/GonkCameraControl.cpp
@@ -1114,6 +1114,10 @@ nsGonkCameraControl::SetPreviewSize(uint32_t aWidth, uint32_t aHeight)

   mWidth = bestWidth;
   mHeight = bestHeight;
+  /* Justin: fixed the previewSize to 640*480 */
+  mWidth = 640;
+  mHeight = 480;
+  /* ---end--- */
   mParams.setPreviewSize(mWidth, mHeight);
   PushParameters();
 }
--
(In reply to JustinLee from comment #2)
> I modified geco/dom/camera/GonkCameraControl.cpp to set preview size to
> 640*480. The performance of camera preview got improvement by this
> modification. The below shows my modification.

Is a valid preview size being passed into getPreviewStream() in camera.js?
(Reporter)

Comment 4

4 years ago
(In reply to Mike Habicher [:mikeh] from comment #3)
> (In reply to JustinLee from comment #2)
> > I modified geco/dom/camera/GonkCameraControl.cpp to set preview size to
> > 640*480. The performance of camera preview got improvement by this
> > modification. The below shows my modification.
> 
> Is a valid preview size being passed into getPreviewStream() in camera.js?

The preview size is calculated by below codes. It seems that preview size can't be set by manual config.

// Previews should match the aspect ratio and not be smaller than the screen
    var validPreviews = camera.capabilities.previewSizes.filter(function(res) {
      var isLarger = res.height >= screenHeight && res.width >= screenWidth;
      var aspectRatio = res.height / res.width;
      var matchesRatio = Math.abs(aspectRatio - pictureAspectRatio) < 0.05;
      return matchesRatio && isLarger;
    });

    // We should always have a valid preview size, but just in case                              
    // we dont, pick the first provided.
    if (validPreviews.length) {
      // Pick the smallest valid preview
      this._previewConfig = validPreviews.sort(function(a, b) {
        return a.width * a.height - b.width * b.height;
      }).shift();
    } else {
      this._previewConfig = camera.capabilities.previewSizes[0];
    }
What does that function say it's setting the preview size to?
Flags: needinfo?(hungche)

Comment 6

4 years ago
Kevin,

FxOS Perf has no Flatfish devices so can you find someone on the Taipei team to own this.

FxOS Perf Triage
Flags: needinfo?(khu)
Keywords: perf
Whiteboard: [c= p= s= u=]

Comment 7

4 years ago
Hi Vincent/Gary,

Could you help to check this case?

Normally Gaia - Camera will choose the resolution which is most close to the panel size.
Maybe the possible preview resolution set passed from Camera HAL didn't fit current panel size.

Thanks.
Sincerely yours.

Comment 8

4 years ago
(In reply to Marco Chen [:mchen] from comment #7)
> Hi Vincent/Gary,
> 
> Could you help to check this case?
> 
> Normally Gaia - Camera will choose the resolution which is most close to the
> panel size.
> Maybe the possible preview resolution set passed from Camera HAL didn't fit
> current panel size.
> 
> Thanks.
> Sincerely yours.

Talked with Gary and he can track this issue.
(Reporter)

Comment 9

4 years ago
(In reply to Mike Habicher [:mikeh] from comment #5)
> What does that function say it's setting the preview size to?

In fact, I'm trying to know why that function does.
Flags: needinfo?(hungche)
(Reporter)

Comment 10

4 years ago
There has a configuration file called "camera.cfg" in flatfish codebase. The file is used for camera setting include preview size, picture size, etc.
About camera preview size setting, it is configured to 2560x1920,1280x720,640x480,1920x1080 by camera.cfg. The preview size has 4 available resolutions on flatfish platform. And camera.cfg also defined a default_preview_size (the setting is 1280x720 now) for default setting.
B2G choices the highest available resolution (2560X1920) for camera previewing. It is the root cause of bad camera preview performance.
Now, I modified the preview size configuration to 640X480 only in camera.cfg. Camera preview performance looks better after that modification.

Comment 11

4 years ago
Three questions:
1. Why B2G doesn't choose to use the default_preview_size, but the largest one instead?
2. It seems that the supported preview size is predefined in camera.cfg. Can it be determined by the system during the run-time (by querying the camera hal?
3. Is there any existing method which can be used to evaluate a proper preview size according to the camera and processor capability?

Updated

4 years ago
blocking-b2g: --- → koi?
Flags: needinfo?(khu)

Comment 12

4 years ago
Hi Gary,

Could you give an explanation for what is the rule to select preview resolution?
Thanks. 

As I know that camera app will iterate all possible resolution and choose one which 
  1. is larger or equal to the screen resolution.
  2. aspect ratio is the same with the screen or it's deviation is less then 5%.
 
  3. if there is no one there then choose first one in the array.
https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L950
Flags: needinfo?(gchen)

Comment 13

4 years ago
I want to explain that Android also use the similar implementation to chose the optimal preview size. Please see getOptimalPreviewSize(...) in the below link.

https://android.googlesource.com/platform/packages/apps/Camera/+/android-4.2.2_r1.2/src/com/android/camera/Util.java

Maybe we can check the method used in Android and compared with our implement in camera.js to find a better way to chose a better resolution. Otherwise, I think the current implementation in FFOS is the reference way to pick up a resolution like the Android did.

Comment 14

4 years ago
(In reply to William Liang from comment #11)
> Three questions:
> 1. Why B2G doesn't choose to use the default_preview_size, but the largest
> one instead?
> 2. It seems that the supported preview size is predefined in camera.cfg. Can
> it be determined by the system during the run-time (by querying the camera
> hal?
> 3. Is there any existing method which can be used to evaluate a proper
> preview size according to the camera and processor capability?

Camera app queries supported preview sizes from camera hal and selects the one that fits screen size best. Please configure supported preview sizes in camera hal if you don't want upper layer app to choose certain sizes. (I think that's the camera.cfg for since neither camera app nor gecko camera look up that file.)

Comment 15

4 years ago
> Could you give an explanation for what is the rule to select preview
> resolution?
> Thanks.
Please refer to bug 866790. The bug intends to solve the inconsistency between preview and postview due to different aspect ratios. The fix is to select preview that matches postview (=picture) aspect ratio.

>   1. is larger or equal to the screen resolution.
>   2. aspect ratio is the same with the screen or it's deviation is less then
> 5%.
Camera app queries supported preview sizes from camera hal and selects sizes that 1) match picture aspect ratio and 2) are larger than screen size to fulfill screen [1]. Then it sorts valid preview sizes from small to large width*height value, and picks the smallest as it already suffices for screen [2].
 
[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L949
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L960
Flags: needinfo?(gchen)

Updated

4 years ago
Whiteboard: [c= p= s= u=] → [c= p= s= u=][Flatfish only][developer-]
Assignee: nobody → gchen
Whiteboard: [c= p= s= u=][Flatfish only][developer-] → [c= p= s= u=]
(Reporter)

Comment 16

4 years ago
(In reply to Ben Tian [:btian] from comment #15)
> > Could you give an explanation for what is the rule to select preview
> > resolution?
> > Thanks.
> Please refer to bug 866790. The bug intends to solve the inconsistency
> between preview and postview due to different aspect ratios. The fix is to
> select preview that matches postview (=picture) aspect ratio.
> 
> >   1. is larger or equal to the screen resolution.
> >   2. aspect ratio is the same with the screen or it's deviation is less then
> > 5%.
> Camera app queries supported preview sizes from camera hal and selects sizes
> that 1) match picture aspect ratio and 2) are larger than screen size to
> fulfill screen [1]. Then it sorts valid preview sizes from small to large
> width*height value, and picks the smallest as it already suffices for screen
> [2].

The screen size is 1280X800 (aspect ratio is 1.6) on faltfish platform. And the supported preview sizes are 2560x1920 ,1280x720,640x480,1920x1080. (the aspect ratio of supported preview sizes are 1.3333, 1.7778, 1.3333, 1.7778.)

I still can't understand that why dose camera app choose 2560X1920 (aspect ratio is 1.3333) for its preview size. And why camera app doesn't choose the default setting of preview size (default_preview_size = 1280X720) or the other resolutions (1920X1080)?

>  
> [1]
> https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L949
> [2]
> https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L960
Hi Justine,
   Here is the log.
   
   screen size 1280X800
   ratio is 1.6

@@@camera - capabilities.previewSizes[
     {"width":2560,"height":1920},  //ratio is 0.75
     {"width":1280,"height":720},   //ratio is 1.77 isLarger = false;
     {"width":640,"height":480},    //ratio is 1.33 isLarger = false;
     {"width":1920,"height":1080}]  //ratio is 0.56 matchesRatio = false;
After filter, we got only one fit on gaia's condition.
https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L949

@@@camera - orign validPreviews[{"width":2560,"height":1920}]
@@@camera _previewConfig{"width":2560,"height":1920}

BTW, I've a patch to fix this issue. :)
(In reply to GaryChen [:GaryChen][:PYChen] from comment #17)
> Hi Justine,
>    Here is the log.
sorry corrected the ration.
>    
>    screen size 1280X800
>    ratio is 1.6
> 
> @@@camera - capabilities.previewSizes[
>      {"width":2560,"height":1920},  //ratio is 0.75
>      {"width":1280,"height":720},   //ratio is 1.77 isLarger = false;
                                      //ratio is 0.56 matchesRatio = false;
>      {"width":640,"height":480},    //ratio is 1.33 isLarger = false;
                                      //ratio is 0.75
>      {"width":1920,"height":1080}]  //ratio is 0.56 matchesRatio = false;
> After filter, we got only one fit on gaia's condition.
> https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L949
> 
> @@@camera - orign validPreviews[{"width":2560,"height":1920}]
> @@@camera _previewConfig{"width":2560,"height":1920}
> 
> BTW, I've a patch to fix this issue. :)
(Reporter)

Comment 19

4 years ago
(In reply to GaryChen [:GaryChen][:PYChen] from comment #18)
> (In reply to GaryChen [:GaryChen][:PYChen] from comment #17)
> > Hi Justine,
> >    Here is the log.
> sorry corrected the ration.
> >    
> >    screen size 1280X800
> >    ratio is 1.6

Is this ratio wrong? Is the right ratio 0.625?

> > 
> > @@@camera - capabilities.previewSizes[
> >      {"width":2560,"height":1920},  //ratio is 0.75

> >      {"width":1280,"height":720},   //ratio is 1.77 isLarger = false;
>                                       //ratio is 0.56 matchesRatio = false;
> >      {"width":640,"height":480},    //ratio is 1.33 isLarger = false;
>                                       //ratio is 0.75
> >      {"width":1920,"height":1080}]  //ratio is 0.56 matchesRatio = false;

Why does 0.75 (2560X1920) match 0.625 (1280X800)?
And why doesn't 0.56 (1920X1080) match 0.625 (1280X800)?

It's hard to understand...

> > After filter, we got only one fit on gaia's condition.
> > https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L949
> > 
> > @@@camera - orign validPreviews[{"width":2560,"height":1920}]
> > @@@camera _previewConfig{"width":2560,"height":1920}
> > 
> > BTW, I've a patch to fix this issue. :)

Thanks for the patch. Where can I find the patch? and what did you do via the patch?

Comment 20

4 years ago
> The screen size is 1280X800 (aspect ratio is 1.6) on faltfish platform. And
> the supported preview sizes are 2560x1920 ,1280x720,640x480,1920x1080. (the
> aspect ratio of supported preview sizes are 1.3333, 1.7778, 1.3333, 1.7778.)
> 
> I still can't understand that why dose camera app choose 2560X1920 (aspect
> ratio is 1.3333) for its preview size. And why camera app doesn't choose the
> default setting of preview size (default_preview_size = 1280X720) or the
> other resolutions (1920X1080)?

screen size:
  1280x800
picture size (aspect ratio = H/W):
  2560x1920(0.75)
preview sizes (aspect ratio = H/W):
 - 2560x1920 (0.75)  [valid]   is larger than screen size AND match picture aspect ratio=0.75
 - 1280x720 (0.56)   [invalid] height < screen height
 - 640x480 (0.75)    [invalid] width < screen width
 - 1920x1080 (0.56)  [invalid] mismatches picture aspect ratio=0.75

=> 2560x1920 is the only valid preview size.

--
Where is default_preview_size=1280X720 stored and who reads it? Camera app or hal?

Comment 21

4 years ago
> Why does 0.75 (2560X1920) match 0.625 (1280X800)?
> And why doesn't 0.56 (1920X1080) match 0.625 (1280X800)?

Please see gaia code [1]. It's the picture size to match, not screen size.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L953
Created attachment 827908 [details]
setPreviewSize.text
Created attachment 827912 [details] [review]
pull request: https://github.com/mozilla-b2g/gaia/pull/13419/

Hi all,
   This is my WIP patch, I use distance of the screenSize(width+height) to validPreviewSize for choosing more approximate valid |previewConfig|.

   Please refer to Android |getOptimalPreviewSize| on #c13, and give me some feedback.

   BTW, I would like to file a new bug for this patch, as :btian mentioned on comment 14, configuring supported preview sizes in 'camera hal' is the best way.
Attachment #827912 - Flags: feedback?(vliu)
Attachment #827912 - Flags: feedback?(btian)
Attachment #827912 - Flags: feedback?(dale)
Comment on attachment 827912 [details] [review]
pull request: https://github.com/mozilla-b2g/gaia/pull/13419/

Obviously the commented code will be removed, but the general approach looks good to me
Attachment #827912 - Flags: feedback?(dale) → feedback+

Comment 25

4 years ago
Comment on attachment 827912 [details] [review]
pull request: https://github.com/mozilla-b2g/gaia/pull/13419/

Looks good to me.

One minor thing that square-root computation for diff is unnecessary as we only care their relative difference.
Attachment #827912 - Flags: feedback?(btian) → feedback+
clear blocking flag until we have clarity for flatfish
blocking-b2g: koi? → ---
See Also: → bug 936400
unassigned, since we won't fix gaia part for this issue.
Assignee: gchen → nobody
Attachment #827912 - Flags: feedback?(vliu)
Priority: -- → P4
Whiteboard: [c= p= s= u=] → [c= p= s= u=][TCP]

Updated

4 years ago
Whiteboard: [c= p= s= u=][TCP] → [c= p= s= u=flatfish] [TCP]

Updated

3 years ago
Whiteboard: [c= p= s= u=flatfish] [TCP] → [c= p= s= u=flatfish] [TCP][flatfish]

Updated

3 years ago
Whiteboard: [c= p= s= u=flatfish] [TCP][flatfish] → [c= p= s= u=flatfish] [TCP=performance][flatfish]

Updated

3 years ago
Blocks: 942087
The camera app and stack have changed significantly since this issue was first filed. Does the viewfinder still have performance issues?
Flags: needinfo?(hungche)
You need to log in before you can comment on or make changes to this bug.