Closed Bug 826756 Opened 7 years ago Closed 7 years ago

Camera - video size mismatch between nsHTMLVideoElement and ImageContainer

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:tef+, blocking-basecamp:+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18+ fixed, b2g18-v1.0.0 fixed)

VERIFIED FIXED
B2G C4 (2jan on)
blocking-b2g tef+
blocking-basecamp +
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 + fixed
b2g18-v1.0.0 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

Details

Attachments

(2 files, 3 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20121231071231

Steps to reproduce:

1. start Camera application
2. change video/camera mode repeatedly by push the mode change button


Actual results:

preview sometimes doesn't fill screen after mode switch


Expected results:

preview should fill screen if camera app did not take a photo
Attached image a screenshot of the problem happened (obsolete) —
blocking-basecamp: --- → ?
Unagi phone, build 2013-01-03
It might not be a defect of gaia, but could be a defect of gecko around nsVideoFrame. nsVideoFrame might have old VideoFrame information. It is just my assumption. I am going to confirm it.
Size of preview image is different between picture mode and video mode in unagi device.
 - preview size of picture mode: w 480 h 320
 - preview size of video mode  : w 352 h 288
Status: UNCONFIRMED → NEW
Ever confirmed: true
blocking-basecamp: ? → -
tracking-b2g18: --- → +
Assignee: nobody → sotaro.ikeda.g
Hardware: All → ARM
OS: All → Gonk (Firefox OS)
I found one problem and one misunderstanding of mime.

[1] video size mismatch between nsHTMLVideoElement and ImageContainer happens.
VideoFrameContainer::Invalidate() skips update of nsHTMLVideoElement's video size, when ImageContainer is async.

[2] When unagi device is in video mode, Video could not fill screen without transform.
Current video tag keeps video aspect ratio.
unagi device's display size is 480* 320 (aspect ratio is 1.5 : 1).
video mode of camera app's of unagi is 352 * 288 (aspect ratio is 1.22 : 1).
Therefore, preview video of video mode do not fill screen.
(In reply to Sotaro Ikeda [:sotaro] from comment #6)
> I found one problem and one misunderstanding of mime.
> 
> [1] video size mismatch between nsHTMLVideoElement and ImageContainer
> happens.
> VideoFrameContainer::Invalidate() skips update of nsHTMLVideoElement's video
> size, when ImageContainer is async.
> 

I also watched following case.
 - receive 352 * 288 frame in camera mode UI
 - receive 480 * 320 frame in video mode UI
[1] mismatch between nsHTMLVideoElement and ImageContainer is kept by following code in VideoFrameContainer::Invalidate().

http://mxr.mozilla.org/mozilla-central/source/content/media/VideoFrameContainer.cpp#101
Summary: Camera - preview sometimes doesn't fill screen after mode switch → Camera - video size mismatch between nsHTMLVideoElement and ImageContainer
Attachment #697968 - Attachment is obsolete: true
Attachment #697970 - Attachment is obsolete: true
Attached patch patch: update video image size (obsolete) — Splinter Review
process VideoFrameContainer::Invalidate() when image size is changed
process VideoFrameContainer::Invalidate() when image size is changed
Attachment #702585 - Attachment is obsolete: true
(In reply to Sotaro Ikeda [:sotaro] from comment #6)

> [2] When unagi device is in video mode, Video could not fill screen without
> transform.
> Current video tag keeps video aspect ratio.
> unagi device's display size is 480* 320 (aspect ratio is 1.5 : 1).
> video mode of camera app's of unagi is 352 * 288 (aspect ratio is 1.22 : 1).
> Therefore, preview video of video mode do not fill screen.

Hi Sotaro,

Is it possible using the below patch like _previewConfig() did? 

diff --git a/apps/camera/js/camera.js b/apps/camera/js/camera.js
index ff6b090..7f59fbb 100644
--- a/apps/camera/js/camera.js
+++ b/apps/camera/js/camera.js
@@ -69,8 +69,8 @@ var Camera = {
   _previewConfigVideo: {
     profile: 'cif',
     rotation: 0,
-    width: 352,
-    height: 288
+    width: document.body.clientHeight,
+    height: document.body.clientWidth
   },
Summary: Camera - video size mismatch between nsHTMLVideoElement and ImageContainer → Camera - preview sometimes doesn't fill screen after mode switch
(In reply to vliu from comment #11)
> (In reply to Sotaro Ikeda [:sotaro] from comment #6)
> 
> > [2] When unagi device is in video mode, Video could not fill screen without
> > transform.
> > Current video tag keeps video aspect ratio.
> > unagi device's display size is 480* 320 (aspect ratio is 1.5 : 1).
> > video mode of camera app's of unagi is 352 * 288 (aspect ratio is 1.22 : 1).
> > Therefore, preview video of video mode do not fill screen.
> 
> Hi Sotaro,
> 
> Is it possible using the below patch like _previewConfig() did? 

I checked it on my unagi phone. But it did not work. It seems that preview size is determined by "profile: 'cif'.
(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> (In reply to vliu from comment #11)
> > (In reply to Sotaro Ikeda [:sotaro] from comment #6)
> > 
> > > [2] When unagi device is in video mode, Video could not fill screen without
> > > transform.
> > > Current video tag keeps video aspect ratio.
> > > unagi device's display size is 480* 320 (aspect ratio is 1.5 : 1).
> > > video mode of camera app's of unagi is 352 * 288 (aspect ratio is 1.22 : 1).
> > > Therefore, preview video of video mode do not fill screen.
> > 
> > Hi Sotaro,
> > 
> > Is it possible using the below patch like _previewConfig() did? 
> 
> I checked it on my unagi phone. But it did not work. It seems that preview
> size is determined by "profile: 'cif'.

I checked the source code and found MediaProfile reads profiles from /etc/media_profiles.xml which our record preview resolution depends on. Then I flashed Android and found that:
  - The SurfaceView size for preview is always 320x427
  - Picture preview size is 384x288 (from log)
  - Record preview size is 320x240 (from log)

I wonder how Android managed to show them without difference :S
I checked my android phone(Optmus LG). When I set a video mode size to QVGA(320*240), preview image do not fill screen. But UI seems more natural.
In this screen shot, layout is for camera mode(480*320), and rendering preview video is video mode(352*288).
Summary: Camera - preview sometimes doesn't fill screen after mode switch → Camera - video size mismatch between nsHTMLVideoElement and ImageContainer
blocking-b2g: --- → tef?
Comment on attachment 702589 [details] [diff] [review]
patch: update video image size

dublec, can you review this patch?
Attachment #702589 - Flags: review?(chris.double)
blocking-basecamp: - → +
Blocking+. In the words of Akeybl, "this is feature phone regression".
blocking-b2g: tef? → tef+
Attachment #702589 - Flags: review?(chris.double) → review+
Keywords: checkin-needed
Component: Gaia::Camera → General
QA Contact: jhammink
https://hg.mozilla.org/mozilla-central/rev/47ad7a78f0b7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
> I checked the source code and found MediaProfile reads profiles from
> /etc/media_profiles.xml which our record preview resolution depends on. Then
> I flashed Android and found that:
>   - The SurfaceView size for preview is always 320x427
>   - Picture preview size is 384x288 (from log)
>   - Record preview size is 320x240 (from log)
> 
> I wonder how Android managed to show them without difference :S

384x288 and 320x240 are same aspect ratio(1.3333), therefore it looks same, I think. And their aspect ratios are different from display size. Preview surface do not fill screen. There is always an opaque ui on right side.
(In reply to Sotaro Ikeda [:sotaro] from comment #21)
> > I checked the source code and found MediaProfile reads profiles from
> > /etc/media_profiles.xml which our record preview resolution depends on. Then
> > I flashed Android and found that:
> >   - The SurfaceView size for preview is always 320x427
> >   - Picture preview size is 384x288 (from log)
> >   - Record preview size is 320x240 (from log)
> > 
> > I wonder how Android managed to show them without difference :S
> 
> 384x288 and 320x240 are same aspect ratio(1.3333), therefore it looks same,
> I think. And their aspect ratios are different from display size. Preview
> surface do not fill screen. There is always an opaque ui on right side.

I tried to trace the Camera app source of AOSP, and I think they do scale + crop to the preview screen (if SurfaceTexture present, in CameraScreenNail.java).

And then I use gdb to attach chrome process, break in ImageLayerOGL::renderLayer, then found the transform applied in record preview will create a 392x320 rectangle on my device with this patch. 

The matrix I got was:
[1.11363637 0 0 0][0 1.11111116 0 0][0 0 1 0][0 0 0 1]

which do not contain rotation, and the scales applied to x,y dimension are not the same. In fact, this should distort the image slightly but hard to observe.

I will try it later, and try to dig out more detail...
Breakpoint 1, mozilla::layers::ShadowImageLayerOGL::RenderLayer (this=0x47deb400, aPreviousFrameBuffer=<value optimized out>, aOffset=...)
    at /home/chiajung/FirefoxOS/b2g-desktop-client/mozilla-central/gfx/layers/opengl/ImageLayerOGL.cpp:1071
1071	    mOGLManager->BindAndDrawQuad(program);
(gdb) l
1066	    program->SetLayerOpacity(GetEffectiveOpacity());
1067	    program->SetRenderOffset(aOffset);
1068	    program->SetTextureUnit(0);
1069	    program->LoadMask(GetMaskLayer());
1070	
1071	    mOGLManager->BindAndDrawQuad(program);
1072	
1073	    // Make sure that we release the underlying external image
1074	    gl()->fActiveTexture(LOCAL_GL_TEXTURE0);
1075	    gl()->fBindTexture(LOCAL_GL_TEXTURE_EXTERNAL, 0);
(gdb) p mTransform
$1 = {_11 = 1.11363637, _12 = 0, _13 = 0, _14 = 0, _21 = 0, _22 = 1.11111116, _23 = 0, _24 = 0, _31 = 0, _32 = 0, _33 = 1, _34 = 0, _41 = 44, _42 = 0, _43 = 0, _44 = 1}
(gdb) p mSize
$2 = {<mozilla::gfx::BaseSize<int, nsIntSize>> = {width = 352, height = 288}, <No data fields>}
(gdb) p aOffset
$3 = (const nsIntPoint &) @0x468cf4d0: {<mozilla::gfx::BasePoint<int, nsIntPoint>> = {x = 0, y = 0}, <No data fields>}
(gdb) p GetEffectiveVisibleRegion
$4 = {const nsIntRegion &(class mozilla::layers::Layer *)} 0x412a6180 <mozilla::layers::Layer::GetEffectiveVisibleRegion()>
(gdb) p GetEffectiveVisibleRegion()
$5 = (const nsIntRegion &) @0x47deb408: {mImpl = {mRectCount = 1, mCurRect = 0x44173424, 
    mRectListHead = {<nsRegion::nsRectFast> = {<nsRect> = {<mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = {x = 0, y = 0, width = 0, height = 0}, <No data fields>}, <No data fields>}, 
      prev = 0x44173424, next = 0x44173424}, mBoundRect = {<nsRect> = {<mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>> = {x = 0, y = 0, width = 352, 
          height = 288}, <No data fields>}, <No data fields>}}}
(gdb) 

here is my gdb log for record preview.
Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.
Verified as fixed. Camera preview displays in a full screen mode,  if camera app did not take a photo. 
Build 20130220070204
Dec 5th Kernel
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/140d06fc3397
Gaia   b8bd76b1110417ef43a39ae28a2eeffbce1eda11
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.