Closed Bug 971531 Opened 6 years ago Closed 6 years ago

tarako camera does not generate proper exif previews

Categories

(Firefox OS Graveyard :: GonkIntegration, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

RESOLVED FIXED
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed

People

(Reporter: bkelly, Assigned: Dafeng.Xu)

Details

(Whiteboard: [POVB])

While using the camera on tarako I saw this in the log:

E/GeckoConsole(11436): Content JS ERROR at app://camera.gaiamobile.org/shared/js/media/media_frame.js:109 in displayImage: The thumbnail contained in the jpeg doesn't fitthe device screen. The full size image is rendered.This might cause out of memory errors

We rely on the camera to provide an exif preview tuned to the device screen size to avoid expensive image manipulation operations in camera and gallery.

We should see what needs to be done to get tarako camera to generate these exif previes.
Ben, do you know if we need to get the OEM to fix the camera EXIF previews or is this something we can do?  Thanks!
Flags: needinfo?(btian)
The max supported jpeg thumbnail size from OEM camera driver (320x240) is smaller than screen size (480x320). ni? James Zhang from OEM.

James, can camera driver increase max supported jpeg thumbnail size to be larger than 480x320? When displaying image, camera app requires jpeg thumbnail size be larger than screen size to save expensive memory usage for upscaling.
Flags: needinfo?(btian) → needinfo?(james.zhang)
(In reply to Ben Tian [:btian] from comment #2)
> The max supported jpeg thumbnail size from OEM camera driver (320x240) is
> smaller than screen size (480x320). ni? James Zhang from OEM.
> 
> James, can camera driver increase max supported jpeg thumbnail size to be
> larger than 480x320? When displaying image, camera app requires jpeg
> thumbnail size be larger than screen size to save expensive memory usage for
> upscaling.
Note camera app requires only one dimension of jpeg thumbnail size be larger than screen size according to [1].

[1] https://github.com/mozilla-b2g/gaia/blob/v1.3t/shared/js/media/media_frame.js#L95
Flags: needinfo?(james.zhang)
Assignee: nobody → Dafeng.Xu
Status: NEW → ASSIGNED
Please apply this patch for a temporary fix.
Not test yet, I am testing it.

diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h b/common/libs/libcamera/sc8810/SprdCameraHardwareC
index b781783..9ce826f 100755
--- a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
+++ b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
@@ -373,7 +373,7 @@ struct config_element sprd_back_camera_hardware_config[] = {
        {"preview-fps-range", "10000,30000"},
        {"jpeg-thumbnail-size-values", "320x240,0x0"},
        {"jpeg-thumbnail-width","320"},
-       {"jpeg-thumbnail-height", "240"},
+       {"jpeg-thumbnail-height", "480"},
        {"jpeg-thumbnail-quality", "80"},
        {"effect-values",
 #ifdef CONFIG_CAMERA_788
triage: 1.3T+ for release
blocking-b2g: 1.3T? → 1.3T+
Whiteboard: [POVB]
test failed

(In reply to ying.xu from comment #4)
> Please apply this patch for a temporary fix.
> Not test yet, I am testing it.
> 
> diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> b/common/libs/libcamera/sc8810/SprdCameraHardwareC
> index b781783..9ce826f 100755
> --- a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> +++ b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> @@ -373,7 +373,7 @@ struct config_element sprd_back_camera_hardware_config[]
> = {
>         {"preview-fps-range", "10000,30000"},
>         {"jpeg-thumbnail-size-values", "320x240,0x0"},
>         {"jpeg-thumbnail-width","320"},
> -       {"jpeg-thumbnail-height", "240"},
> +       {"jpeg-thumbnail-height", "480"},
>         {"jpeg-thumbnail-quality", "80"},
>         {"effect-values",
>  #ifdef CONFIG_CAMERA_788
please apply this patch ,tested OK

diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h b/common/libs/libcamera/sc8810/SprdCameraHardwareC
index b781783..4a4b4f9 100755
--- a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
+++ b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
@@ -371,9 +371,9 @@ struct config_element sprd_back_camera_hardware_config[] = {
        {"preview-frame-rate", "20"},
        {"preview-fps-range-values", "(10000,30000)"},
        {"preview-fps-range", "10000,30000"},
-       {"jpeg-thumbnail-size-values", "320x240,0x0"},
+       {"jpeg-thumbnail-size-values", "320x480,0x0"},
        {"jpeg-thumbnail-width","320"},
-       {"jpeg-thumbnail-height", "240"},
+       {"jpeg-thumbnail-height", "480"},
        {"jpeg-thumbnail-quality", "80"},
        {"effect-values",
 #ifdef CONFIG_CAMERA_788
Ying,

The new aspect ratio (2:3) is inconsistent with original one (4:3) and may distort the thumbnail. Have you checked the generated thumbnail? Also how much does jpeg encoder's memory usage increase? We should ensure Tarako allows the additional memory usage.

(In reply to ying.xu from comment #7)
> please apply this patch ,tested OK
> 
> diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> b/common/libs/libcamera/sc8810/SprdCameraHardwareC
> index b781783..4a4b4f9 100755
> --- a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> +++ b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> @@ -371,9 +371,9 @@ struct config_element sprd_back_camera_hardware_config[]
> = {
>         {"preview-frame-rate", "20"},
>         {"preview-fps-range-values", "(10000,30000)"},
>         {"preview-fps-range", "10000,30000"},
> -       {"jpeg-thumbnail-size-values", "320x240,0x0"},
> +       {"jpeg-thumbnail-size-values", "320x480,0x0"},
>         {"jpeg-thumbnail-width","320"},
> -       {"jpeg-thumbnail-height", "240"},
> +       {"jpeg-thumbnail-height", "480"},
>         {"jpeg-thumbnail-quality", "80"},
>         {"effect-values",
>  #ifdef CONFIG_CAMERA_788
Flags: needinfo?(ying.xu)
Flags: needinfo?(ying.xu)
Flags: needinfo?(Dafeng.Xu)
(In reply to Ben Tian [:btian] from comment #8)
> Ying,
> 
> The new aspect ratio (2:3) is inconsistent with original one (4:3) and may
> distort the thumbnail. Have you checked the generated thumbnail?

I didn't check this before. 

> much does jpeg encoder's memory usage increase? We should ensure Tarako
> allows the additional memory usage.

thumbnail generating sequence:

sensor raw data(YUV big) -----> thumbnail data(YUV) ----> thumbnail jpg

Basically, it will consume memory to hold the thumbnail.  That's not too much in contrast with capturing buffer.

Because the ratio problem between the original picture and thumbnail one.

If we keep the ratio the same between them 

we can change thumbnail size bigger, but use more memory.

or we change the size of capture picture , That will involved more modifications(driver, maybe gecko/gaia).

Besides, the pictures with thumbnail that were not taken from camera would have this problem if thumbnail size was smaller than screen size.

So we will not fix this.
Its going to make it difficult for us to have a performant camera app on v1.3 without this bug getting fixed.  Right now we require large amounts of memory to resize images in web contact (like our apps).  Hopefully that will get fixed with bug 854795, but that won't be ready for v1.3.  Our other OEM partners have helped here by generating EXIF thumbnails at least as large as the screen.

David, is that a fair assessment of the situation?
Flags: needinfo?(dflanagan)
Yes, that's a fair assessment.

The latest change is bug 942199 where we've added a build-time configuration option to support a QC device that can't generate previews that are quite big enough.  If we can get a 3:4 thumbnail that is almost big enough (like 300x400), we can use a build-time configuration to treat that as good enough and accept a slight visual degradation in previews in exchange for much improved performance.  So being as big as the screen size is no longer a hard requirement.  But we do need a custom gaia build to relax that size requirement

The EXIF preview does need to match the aspect ratio of the picture. That is a hard requirement.

I don't understand Ying Xu's response in comment #9. Can't the driver support 3:4 thumbnails?  What sizes are available?
Flags: needinfo?(dflanagan)
Ying Xu, is it possible to generate a thumbnail in the 3:4 aspect ratio that is almost as big as the screen?  If so, it appears we can compensate with a custom gaia build.
Flags: needinfo?(ying.xu)
(In reply to Ben Kelly [:bkelly] from comment #12)
> Ying Xu, is it possible to generate a thumbnail in the 3:4 aspect ratio that
> is almost as big as the screen?  If so, it appears we can compensate with a
> custom gaia build.

that's a size 360X480, bigger than the whole screen.
we can do it without increasing too much size for thumbnail of 3:2 aspect ratio(720X480)

so you're going use a 360X480 size thumbnail to generate a new one of 3:2 aspect ratio ?
(In reply to ying.xu from comment #13)
> that's a size 360X480, bigger than the whole screen.
> we can do it without increasing too much size for thumbnail of 3:2 aspect
> ratio(720X480)
Great. Please provide us a patch to test and let us know once it lands on your codebase.

> so you're going use a 360X480 size thumbnail to generate a new one of 3:2
> aspect ratio ?
Yes. Driver only needs to generate a 480x360 thumbnail that 1) is larger than screen size 480x320 and 2) keeps aspect ratio 4:3. Camera app will fit the 4:3 thumbnail onto 3:2 screen.
(In reply to Ben Tian [:btian] from comment #14)
> (In reply to ying.xu from comment #13)
> > that's a size 360X480, bigger than the whole screen.
> > we can do it without increasing too much size for thumbnail of 3:2 aspect
> > ratio(720X480)
> Great. Please provide us a patch to test and let us know once it lands on
> your codebase.

patch

diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h b/common/libs/libcamera/sc8810/SprdCameraHardwareC
index b781783..0c290d2 100755
--- a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
+++ b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
@@ -371,9 +371,9 @@ struct config_element sprd_back_camera_hardware_config[] = {
        {"preview-frame-rate", "20"},
        {"preview-fps-range-values", "(10000,30000)"},
        {"preview-fps-range", "10000,30000"},
-       {"jpeg-thumbnail-size-values", "320x240,0x0"},
-       {"jpeg-thumbnail-width","320"},
-       {"jpeg-thumbnail-height", "240"},
+       {"jpeg-thumbnail-size-values", "360x480,0x0"},
+       {"jpeg-thumbnail-width","360"},
+       {"jpeg-thumbnail-height", "480"},
        {"jpeg-thumbnail-quality", "80"},
        {"effect-values",
 #ifdef CONFIG_CAMERA_788

when I tried ,I got these log and the thumbnail seem wrong
E/GeckoConsole(  445): Content JS ERROR at app://camera.gaiamobile.org/gaia_build_defer_index.js:87 in Camera.selectThumbnailSize: Error while selecting thumbnail size. There are no thumbnail sizes that match the ratio of the selected picture size: {"width":1600,"height":1200}
Flags: needinfo?(ying.xu)
(In reply to ying.xu from comment #15)
> (In reply to Ben Tian [:btian] from comment #14)
> > (In reply to ying.xu from comment #13)
> > > that's a size 360X480, bigger than the whole screen.
> > > we can do it without increasing too much size for thumbnail of 3:2 aspect
> > > ratio(720X480)
> > Great. Please provide us a patch to test and let us know once it lands on
> > your codebase.
> 
> patch
> 
> diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> b/common/libs/libcamera/sc8810/SprdCameraHardwareC
> index b781783..0c290d2 100755
> --- a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> +++ b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
> @@ -371,9 +371,9 @@ struct config_element sprd_back_camera_hardware_config[]
> = {
>         {"preview-frame-rate", "20"},
>         {"preview-fps-range-values", "(10000,30000)"},
>         {"preview-fps-range", "10000,30000"},
> -       {"jpeg-thumbnail-size-values", "320x240,0x0"},
> -       {"jpeg-thumbnail-width","320"},
> -       {"jpeg-thumbnail-height", "240"},
> +       {"jpeg-thumbnail-size-values", "360x480,0x0"},
> +       {"jpeg-thumbnail-width","360"},
> +       {"jpeg-thumbnail-height", "480"},
>         {"jpeg-thumbnail-quality", "80"},
>         {"effect-values",
>  #ifdef CONFIG_CAMERA_788
> 
> when I tried ,I got these log and the thumbnail seem wrong
> E/GeckoConsole(  445): Content JS ERROR at
> app://camera.gaiamobile.org/gaia_build_defer_index.js:87 in
> Camera.selectThumbnailSize: Error while selecting thumbnail size. There are
> no thumbnail sizes that match the ratio of the selected picture size:
> {"width":1600,"height":1200}

Please revise the patch as following to match original aspect ratio:

> -       {"jpeg-thumbnail-size-values", "320x240,0x0"},
> -       {"jpeg-thumbnail-width","320"},
> -       {"jpeg-thumbnail-height", "240"},
> +       {"jpeg-thumbnail-size-values", "480x360,0x0"},
> +       {"jpeg-thumbnail-width","480"},
> +       {"jpeg-thumbnail-height", "360"},
Ying, does the revision in comment 16 work? Do you meet any other error?
Flags: needinfo?(ying.xu)
(In reply to Ben Tian [:btian] from comment #17)
> Ying, does the revision in comment 16 work? Do you meet any other error?

It works. I see no errors.
Flags: needinfo?(ying.xu)
Flags: needinfo?(Dafeng.Xu)
(In reply to ying.xu from comment #18)
> (In reply to Ben Tian [:btian] from comment #17)
> > Ying, does the revision in comment 16 work? Do you meet any other error?
> 
> It works. I see no errors.

Should be 480x320.
(In reply to James Zhang from comment #19)
> (In reply to ying.xu from comment #18)
> > (In reply to Ben Tian [:btian] from comment #17)
> > > Ying, does the revision in comment 16 work? Do you meet any other error?
> > 
> > It works. I see no errors.
> 
> Should be 480x320.

James, what do you mean?
Do you mean screen size or thumbnail size should be 480x320? And the reason?
Flags: needinfo?(james.zhang)
It will cause camera take picture bug, Ying will track it.
Flags: needinfo?(james.zhang) → needinfo?(ying.xu)
(In reply to James Zhang from comment #21)
> It will cause camera take picture bug, Ying will track it.
Ying, please explain the bug in detail or let me know the bug number.
James, could you check if there's any update on your side?
Flags: needinfo?(james.zhang)
In device/sprd

commit 37580d325f6500b65c83b989c906bcc8796d3483
Author: dafeng.xu <dafeng.xu@spreadtrum.com>
Date:   Mon Mar 3 16:05:05 2014 +0800

    Bug #281142 the screen reflash is not fast when open picture in camera app
    
    [bug number  ] 281142
    [root cause  ] the thumbnail size is not match the screen size
    [changes     ] change the thumbnail size
    [side effects] none
    [reviewers   ] james.zhang
    
    Change-Id: I2e18c11de24e4d1fb7fe0d2d10fa57f0be19216f

diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
index b781783..1dff32f 100755
--- a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
+++ b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
@@ -371,9 +371,9 @@ struct config_element sprd_back_camera_hardware_config[] = {
        {"preview-frame-rate", "20"},
        {"preview-fps-range-values", "(10000,30000)"},
        {"preview-fps-range", "10000,30000"},
-       {"jpeg-thumbnail-size-values", "320x240,0x0"},
-       {"jpeg-thumbnail-width","320"},
-       {"jpeg-thumbnail-height", "240"},
+       {"jpeg-thumbnail-size-values", "480x320,0x0"},
+       {"jpeg-thumbnail-width","480"},
+       {"jpeg-thumbnail-height", "320"},
        {"jpeg-thumbnail-quality", "80"},
        {"effect-values",
 #ifdef CONFIG_CAMERA_788
Flags: needinfo?(james.zhang)
(In reply to Ben Tian [:btian] from comment #22)
> (In reply to James Zhang from comment #21)
> > It will cause camera take picture bug, Ying will track it.
> Ying, please explain the bug in detail or let me know the bug number.

James/Ying, can you explain the camera bug in detail? The 480x320 (3:2) thumbnail doesn't match picture aspect ratio 4:3 and would result in distorted thumbnail.
Flags: needinfo?(james.zhang)
The thumbnail is gray when the value is 480x360, do you test this value?
Flags: needinfo?(james.zhang)
is it still 480x360 after you revert camera back to 2MP? Thanks
Flags: needinfo?(james.zhang)
(In reply to Joe Cheng [:jcheng] from comment #27)
> is it still 480x360 after you revert camera back to 2MP? Thanks

I mean 480x320 will cause image thumbnail display bug, and we fix this cause by 480x320.
Please find our commit in device/sprd.

commit 37580d325f6500b65c83b989c906bcc8796d3483
Author: dafeng.xu <dafeng.xu@spreadtrum.com>
Date:   Mon Mar 3 16:05:05 2014 +0800

    Bug #281142 the screen reflash is not fast when open picture in camera app
    
    [bug number  ] 281142
    [root cause  ] the thumbnail size is not match the screen size
    [changes     ] change the thumbnail size
    [side effects] none
    [reviewers   ] james.zhang
    
    Change-Id: I2e18c11de24e4d1fb7fe0d2d10fa57f0be19216f

diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
index b781783..1dff32f 100755
--- a/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
+++ b/common/libs/libcamera/sc8810/SprdCameraHardwareConfig.h
@@ -371,9 +371,9 @@ struct config_element sprd_back_camera_hardware_config[] = {
        {"preview-frame-rate", "20"},
        {"preview-fps-range-values", "(10000,30000)"},
        {"preview-fps-range", "10000,30000"},
-       {"jpeg-thumbnail-size-values", "320x240,0x0"},
-       {"jpeg-thumbnail-width","320"},
-       {"jpeg-thumbnail-height", "240"},
+       {"jpeg-thumbnail-size-values", "480x320,0x0"},
+       {"jpeg-thumbnail-width","480"},
+       {"jpeg-thumbnail-height", "320"},
        {"jpeg-thumbnail-quality", "80"},
        {"effect-values",
 #ifdef CONFIG_CAMERA_788
Flags: needinfo?(james.zhang)
Sorry, I mean 480x360 has bug, and 480x320 has no bug.
We use 480x320 now.
Flags: needinfo?(ying.xu)
James, do you mean this bug is now fixed on your side? Thanks
Flags: needinfo?(james.zhang)
(In reply to Joe Cheng [:jcheng] from comment #30)
> James, do you mean this bug is now fixed on your side? Thanks

Yes.
Flags: needinfo?(james.zhang)
Thanks James, please resolved fixed the bug when you have fixed it. thank you
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.