Firefox v1.0.1 Mozilla build ID: 20130304070202. +++ This bug was initially created as a clone of Bug #419477 +++ Created an attachment (id=363966) Screen shot DEFECT DESCRIPTION: (1)Play some videos, you will find the bottom part is stretched a little , no matter in portrait mode or Landscape mode.---- NOK (2)Samples to check: https://www.youtube.com/watch?v=NeGe7lVrXb0&feature=youtube_gdata_player https://www.dropbox.com/s/bxgyyrcvyymi4xd/AvatarMovieTrailer.mp4 REPRODUCING PROCEDURES: EXPECTED BEHAVIOUR: ASSOCIATE SPECIFICATION: TEST PLAN REFERENCE: TOOLS AND PLATFORMS USED: USER IMPACT: big REPRODUCING RATE: 100% For FT PR, Please list reference mobile's behavior: ++++++++++ end of initial bug #419477 description ++++++++++ CONTACT INFO (Name,Phone number): DEFECT DESCRIPTION: REPRODUCING PROCEDURES: EXPECTED BEHAVIOUR: ASSOCIATE SPECIFICATION: TEST PLAN REFERENCE: TOOLS AND PLATFORMS USED: USER IMPACT: REPRODUCING RATE: For FT PR, Please list reference mobile's behavior:

Inder, can you please look at this?

I've seen this before. It seems it's only a problem when Hardware composer is disabled

Does this need to remain a blocker, given comment 3 and our planned config around hardware composer?

(ni on dwilson to further qualify comment 3)

diego, I could reproduce this issue on our device even if hw composer is enabled.

(Back to tef+ for now, until Inder is able to triage why these media file are causing trouble.)

For AvatarMovieTrailer found that there is a discrepancy between the video dimension in the metadata and what decoder finds. In the metadata the video dimensions are 480x200 whereas the decoder finds it as 480x208. Do we know how and where we discover the video dimension in gecko? I am thinking it gets it from decoder.

CCing some media superstars for comment 8.

There are defect's of handling of SurfaceDescriptorGralloc's image size. SurfaceDescriptorGralloc uses android::GraphicBuffer's Image size as descriptor's image's size. It seems that there are cases that GraphicBuffer's size is different from valid Image size. "208" comes from GraphicBuffer's size.

(In reply to Sotaro Ikeda [:sotaro] from comment #10) > There are defect's of handling of SurfaceDescriptorGralloc's image size. > SurfaceDescriptorGralloc uses android::GraphicBuffer's Image size as > descriptor's image's size. It seems that there are cases that > GraphicBuffer's size is different from valid Image size. > > "208" comes from GraphicBuffer's size. I am going to investigate the problem more tomorrow.

I fixed the problem locally. I am going to write a cleaner patch. The problem happens from the size difference between GraphicBuffer's size and actual video size. When there is the size difference, it is necessary to draw by using LayerManagerOGL::BindAndDrawQuadWithTextureRect(). current implementation use LayerManagerOGL::BindAndDrawQuad().

Comment on attachment 729792 [details] [diff] [review] Part1 - add image size to SurfaceDescriptorGralloc :nical, can you review the patch?

Comment on attachment 729793 [details] [diff] [review] Part2 - render gralloc buffer by using image size :roc, can you review the patch?

Comment on attachment 729794 [details] [diff] [review] Part3 - set video size to SurfaceDescriptorGralloc :doublec, can you review the patch?

I confirmed the patches fixed the problem on unagi phone.

Comment on attachment 729794 [details] [diff] [review] Part3 - set video size to SurfaceDescriptorGralloc Review of attachment 729794 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/omx/OmxDecoder.cpp @@ +528,5 @@ > descriptor = mNativeWindow->getSurfaceDescriptorFromBuffer(mVideoBuffer->graphicBuffer().get()); > } > > if (descriptor) { > + // Change the descriptor's size to video's sizse. There are cases that Spelling of 'sizse'.

Comment on attachment 729792 [details] [diff] [review] Part1 - add image size to SurfaceDescriptorGralloc Review of attachment 729792 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/LayersSurfaces.ipdlh @@ +45,5 @@ > }; > > struct SurfaceDescriptorGralloc { > PGrallocBuffer buffer; > + nsIntSize size; Please add a comment here explaining why we store an extra size member instead of just using the android::GraphicBuffer's Image size.

Commitable patch. carry "nical.bugzilla: review+".

Commitable patch. Carry "roc: review+".

Committable patch. Carry "chris.double: review+".

https://hg.mozilla.org/integration/mozilla-inbound/rev/345246c6d010 https://hg.mozilla.org/integration/mozilla-inbound/rev/ae71a585726f https://hg.mozilla.org/integration/mozilla-inbound/rev/6ea012d36e73

https://hg.mozilla.org/mozilla-central/rev/345246c6d010 https://hg.mozilla.org/mozilla-central/rev/ae71a585726f https://hg.mozilla.org/mozilla-central/rev/6ea012d36e73

Is there a gaia component to this patch?

(In reply to John Ford [:jhford] from comment #28) > Is there a gaia component to this patch? IMHO, gaia side patch is not necessary.

Environmental Variables: Unagi Build ID: 20130401070203 Kernel Date: Dec 5 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/b28463f2e718 Gaia: ddb38ac8a34f9e30e09d0ff3b5c1bfb9b664b7c3 The same issue still reproduces on "Unagi" device.

(In reply to Ryan VanderMeulen [:RyanVM] from comment #26) > https://hg.mozilla.org/integration/mozilla-inbound/rev/345246c6d010 > https://hg.mozilla.org/integration/mozilla-inbound/rev/ae71a585726f > https://hg.mozilla.org/integration/mozilla-inbound/rev/6ea012d36e73 Can someone uplift the patches?

Comment on attachment 730261 [details] [diff] [review] Part1 v2 - add image size to SurfaceDescriptorGralloc NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): N/A User impact if declined:Some videos are not rendered correctly Testing completed: Tested on Unagui phone. Risk to taking this patch (and alternatives if risky): low risk String or UUID changes made by this patch: none

(In reply to Sotaro Ikeda [:sotaro] from comment #32) > Comment on attachment 730261 [details] [diff] [review] > Part1 v2 - add image size to SurfaceDescriptorGralloc > > NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to > better understand the B2G approval process and landings. > > [Approval Request Comment] > Bug caused by (feature/regressing bug #): N/A > User impact if declined:Some videos are not rendered correctly > Testing completed: Tested on Unagui phone. > Risk to taking this patch (and alternatives if risky): low risk > String or UUID changes made by this patch: none This bug is already tef+ so no need for approval to uplift the patches.

Patch for b2g18. Carry "nical.bugzilla: review+"

Patch for b2g18. Carry "roc: review+"

Patch for b2g18. Carry "chris.double: review+"

Can I get help landing on gecko b2g18?

https://hg.mozilla.org/releases/mozilla-b2g18/rev/83276ffa8089 https://hg.mozilla.org/releases/mozilla-b2g18/rev/4e2faaa4b3dc https://hg.mozilla.org/releases/mozilla-b2g18/rev/cb21c4153afa https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/a49d901c6f10 https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/f4e509cb83cc https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/fff03f1901f2

Working fine in ikura device with: Gecko http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/2b44e2c40cc1 Gaia 3e7dda88950ae09166109783adea8181eb857d21 BuildID 20130414230203 Version 18.0

Tested on the new release: AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.085 Firefox os v1.0.1 Mozilla build ID:20130422230201 The result is NOK, so I reopen the PR. The problem is still there, no improvment.

Amelie, There is a separater patch for hardware composer, which is enabled in that build. Please see bug 865112.

Diego, So you mean this patch? https://www.codeaurora.org/gitweb/quic/lf/?p=build.git;a=blob;f=patch/all/gecko/HWC-multirect-visible-region.patch;h=1e1f6bde3a648dceb9d3a7f8bff27054e2030936;hb=refs/heads/v1.1 Are you sure it is uplifted to v1.0.1? And we can verify the problem in later build?(after 4/25?) Amelie

Amelie, All pending HWC patches are currently in bug 832383, including the patch that fixes the stretch issue. The plan is to uplift them to 1.0.1. I cc'ed you in the bug. Let's continue the discussion there.