MPEG4 videos of some resolutions display with a green bar

RESOLVED FIXED in Firefox 42

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: diego, Assigned: sotaro)

Tracking

unspecified
mozilla42
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, firefox40 wontfix, firefox41 wontfix, firefox42 fixed, b2g-v2.2 fixed, b2g-v2.2r fixed, b2g-master fixed)

Details

(Whiteboard: [caf priority: p3][CR 811823])

Attachments

(8 attachments, 13 obsolete attachments)

1.00 MB, image/png
Details
89.20 KB, text/plain
Details
443.08 KB, text/plain
Details
5.58 KB, patch
nical
: review+
Details | Diff | Splinter Review
5.64 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
9.72 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
61.56 KB, text/plain
Details
10.10 KB, patch
Details | Diff | Splinter Review
Reporter

Description

4 years ago
There is an issue in the composite layers' ImageHost that improperly renders video buffers when the video content width is not the same as the video buffer width.

I have a patch incoming.
Reporter

Comment 1

4 years ago
There seems to be a rounding error while scaling an image that is smaller than it's containing buffer
Reporter

Updated

4 years ago
blocking-b2g: --- → 2.2?
Whiteboard: [CR 811823]
Whiteboard: [CR 811823] → [caf priority: p3][CR 811823]
Reporter

Updated

4 years ago
Attachment #8609679 - Flags: review?(sotaro.ikeda.g)
Assignee

Comment 2

4 years ago
Comment on attachment 8609679 [details] [diff] [review]
Snap buffer scaling to integer pixels

Review of attachment 8609679 [details] [diff] [review]:
-----------------------------------------------------------------

I am not sure that it is OK to do it on all platforms. Forward the review to matt.woodrow.
Attachment #8609679 - Flags: review?(sotaro.ikeda.g) → review?(matt.woodrow)
Reporter

Comment 3

4 years ago
This is possibly related to this

https://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/CompositorOGL.cpp#1064

I can ifdef it to Android/Gonk as well to reduce the risk too.
This is almost certainly because of bilinear interpolation when drawing the scaled video.

When drawing a scaled texture, this usually results in us sampling pixels outside of the texture coordinate region. When the texture coord rect lies on the edge of the texture then this is fine because sampling is clamped to the texture bounds, but when we have a picture rect we don't get clamping to this edge, so we can sample outside of it.

It's pretty common for areas outside of the picture rect to contain green pixels, so this is the source of the green fringe on the rendered video.

Switching to POINT (nearest neighbour sampling) stops us from sampling outside the picture rect, but it also stops interpolation so the quality of scaled video is going to be a lot worse. I really don't think we want to do that, especially not as a general solution.

Where do these textures come from? Is it possible for us to only upload the picture rect to the texture, rather than the entire image?

The other option is to do hard clamping to the texture coords using shaders, which is something we've talked about before but never needed to implement.
Flags: needinfo?(bas)
(In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> This is almost certainly because of bilinear interpolation when drawing the
> scaled video.
> 
> When drawing a scaled texture, this usually results in us sampling pixels
> outside of the texture coordinate region. When the texture coord rect lies
> on the edge of the texture then this is fine because sampling is clamped to
> the texture bounds, but when we have a picture rect we don't get clamping to
> this edge, so we can sample outside of it.
> 
> It's pretty common for areas outside of the picture rect to contain green
> pixels, so this is the source of the green fringe on the rendered video.

Well, they won't 'really' contain green, it's just that Y & Cb Cr all being 0 resolve to green I believe, right?

> Switching to POINT (nearest neighbour sampling) stops us from sampling
> outside the picture rect, but it also stops interpolation so the quality of
> scaled video is going to be a lot worse. I really don't think we want to do
> that, especially not as a general solution.
> 
> Where do these textures come from? Is it possible for us to only upload the
> picture rect to the texture, rather than the entire image?

Obviously an option but I don't know how well this combines with HW accel decoding where we use texture sharing?

> The other option is to do hard clamping to the texture coords using shaders,
> which is something we've talked about before but never needed to implement.

Shouldn't be a hard problem I'd say, we're using a special shader for these anyway, might as well pass the picture rect in as a shader constant and clamp.
Flags: needinfo?(bas)
Reporter

Comment 6

4 years ago
(In reply to Bas Schouten (:bas.schouten) from comment #5)
> > Where do these textures come from? Is it possible for us to only upload the
> > picture rect to the texture, rather than the entire image?
> 
> Obviously an option but I don't know how well this combines with HW accel
> decoding where we use texture sharing?

Agreed. We get this texture from a HW codec and I don't know if we can do anything about the picture rect/texture rect without a copy.

> > The other option is to do hard clamping to the texture coords using shaders,
> > which is something we've talked about before but never needed to implement.
> 
> Shouldn't be a hard problem I'd say, we're using a special shader for these
> anyway, might as well pass the picture rect in as a shader constant and
> clamp.

I see. Who do you think would be the right person to take a stab at this?
Flags: needinfo?(bas)
Sotaro, this has as potential of becoming 2.2+, and QC is tracking it, can you follow up when you have a chance?
Flags: needinfo?(bas) → needinfo?(sotaro.ikeda.g)
Assignee

Comment 8

4 years ago
Okey, I take a look.
Assignee: dwilson → sotaro.ikeda.g
Flags: needinfo?(sotaro.ikeda.g)
Comment on attachment 8609679 [details] [diff] [review]
Snap buffer scaling to integer pixels

Review of attachment 8609679 [details] [diff] [review]:
-----------------------------------------------------------------

r- based on the discussion above.
Attachment #8609679 - Flags: review?(matt.woodrow) → review-
Assignee

Comment 10

4 years ago
The problem seems to happen in the following case.
 - Video size(width, height) is not equal to decoded buffer size.
 - buffer data of out side of valid video area are filled with 0.
Assignee

Comment 11

4 years ago
But the symptom seems strange. In the past, when the video layer is bigger than video size, the content was stretched on codeaurora platform. It is like Bug 850566.
Assignee

Comment 12

4 years ago
diego, can you provide a content and screenshot of the symptom?
Flags: needinfo?(dwilson)
Assignee

Comment 13

4 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> diego, can you provide a content and screenshot of the symptom?

And does the problem happens on OpenGL rendering only?
Assignee

Comment 14

4 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #11)
> But the symptom seems strange. In the past, when the video layer is bigger
> than video size, the content was stretched on codeaurora platform. It is
> like Bug 850566.

I locally modified the code to intentionally caused the symptom on several devices, I saw that the contents were stretched or wrapped contents were shown. I did not saw green line.
Reporter

Comment 15

4 years ago
Posted image Screenshot
Sotaro,

Please find a screenshot attached.
Flags: needinfo?(dwilson)

Updated

4 years ago
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 16

4 years ago
I tried to reproduce the problem again. Within my devices, only hamachi and dolphin could cause the following. And on those devices, Even when I tried to render whole buffers by modifying the code,I could not see the green line. It seems that hidden low layer's code handle this case.
- video buffers width and valid video size are different.
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 17

4 years ago
I checked android's code, android render video buffer by using GL_LINEAR instead of GL_NEAREST. Therefore using gfx::Filter::POINT to fix the problem seems to cause rendering quality regression compared to android.

Android's media framework set NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW to ANativeWindow. It result to GL_LINEAR usage in Layer::onDraw() in SurfaceFlinger. The following is related code. NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW affects to isFixedSize().

> const bool useFiltering = getFiltering() || needsFiltering(hw) || isFixedSize();
  http://androidxref.com/5.1.0_r1/xref/frameworks/native/services/surfaceflinger/Layer.cpp#630
Assignee

Comment 18

4 years ago
I checked android source code further.

Since android KK, android EGL defines EGL_ANDROID_image_crop extension. When the device supports it, EGLImage could handle crop directly. It is handled in GLConsumer::EglImage::createImage().
  http://androidxref.com/5.1.0_r1/xref/frameworks/native/libs/gui/GLConsumer.cpp#1113

If the device does not support EGL_ANDROID_image_crop, mFilteringEnabled(scaling enabled as in comment 17) , yuv buffer and crop is valid, the crop rectangle is shrunk by 2 texels in each dimension.
  http://androidxref.com/5.1.0_r1/xref/frameworks/native/libs/gui/GLConsumer.cpp#785

Similar handling seems necessary on b2g gonk.
Assignee

Comment 19

4 years ago
Diego, your device supports EGL_ANDROID_image_crop extension?
Flags: needinfo?(dwilson)
Assignee

Comment 20

4 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #19)
> Diego, your device supports EGL_ANDROID_image_crop extension?

Does your device support EGL_ANDROID_image_crop extension?
Reporter

Comment 21

4 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #20)
> (In reply to Sotaro Ikeda [:sotaro] from comment #19)
> > Diego, your device supports EGL_ANDROID_image_crop extension?
> 
> Does your device support EGL_ANDROID_image_crop extension?

It does seem like adreno200 devices support it.
Flags: needinfo?(dwilson)
Assignee

Comment 22

4 years ago
We need to update GLConsts.h as to add the followings.

> 0x3148               EGL_IMAGE_CROP_LEFT_ANDROID (EGL_ANDROID_image_crop)
> 0x3149               EGL_IMAGE_CROP_TOP_ANDROID (EGL_ANDROID_image_crop)
> 0x314A               EGL_IMAGE_CROP_RIGHT_ANDROID (EGL_ANDROID_image_crop)
> 0x314B               EGL_IMAGE_CROP_BOTTOM_ANDROID (EGL_ANDROID_image_crop)

GLParseRegistryXML.py generates GLConsts.h. GLParseRegistryXML.py says that GLConsts.h is generated by using gl.xml, egl.xml, glx.xml and wgl.xml from http://www.opengl.org/registry/#specfiles. But egl.xml does not have  the above EGL_IMAGE_CROP_***_ANDROIDs. They are written only android source code like the following.
  http://androidxref.com/5.1.0_r1/xref/frameworks/native/opengl/specs/README#17
Assignee

Comment 23

4 years ago
Assignee

Comment 24

4 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #23)
> Created attachment 8625576 [details] [diff] [review]
> patch - Necessary change to egl.xml

If EGL_IMAGE_CROP_***_ANDROIDs does not in egl.xml, GLDefs.h seems a correct place to add them.
Assignee

Updated

4 years ago
Attachment #8625576 - Attachment is obsolete: true
Assignee

Comment 26

4 years ago
Comment on attachment 8625605 [details] [diff] [review]
patch - Add EGL_ANDROID_image_crop support

diego, can you confirm if the patch works.
Attachment #8625605 - Flags: feedback?(dwilson)
Any updates on this bug?
Flags: needinfo?(dwilson)

Comment 29

4 years ago
Ryan - apologies for the delay, we are verifying it and will update shortly.
Flags: needinfo?(dwilson) → needinfo?(ikumar)

Comment 30

4 years ago
I have tested the build with patch in comment 27 and it does not fix the issue, where as the fix in comment 1 by Diego does fix it.

Updated

4 years ago
Flags: needinfo?(ikumar)
Assignee

Comment 31

4 years ago
(In reply to Yogendra from comment #30)
> I have tested the build with patch in comment 27 and it does not fix the
> issue, where as the fix in comment 1 by Diego does fix it.

We can not use Diego's patch, it degrade video rendering quality.
Assignee

Comment 32

4 years ago
Posted patch log patch for b2g v2.2 (obsolete) — Splinter Review
Assignee

Comment 33

4 years ago
Inder, Yogendra, can you taka a logcat log by applying attachment 8632817 [details] [diff] [review]?
Flags: needinfo?(yogendra)
Flags: needinfo?(ikumar)

Comment 34

4 years ago
Flags: needinfo?(yogendra)

Comment 35

4 years ago
logs are taken by playing rtsp link: rtsp://203.35.158.135/Brock_25k.3gp
Flags: needinfo?(ikumar)
Assignee

Comment 36

4 years ago
(In reply to Yogendra from comment #34)
> Created attachment 8633356 [details]
> logs taken with build by appyling patch 8632817

Thanks for the log. From it, the hardware supports EglAndroidImageCrop. But from android::GraphicBuffer level, gralloc buffer size and video size are same.
Assignee

Comment 37

4 years ago
On current patch, EglAndroidImageCrop is set only when android::GraphicBuffer and video buffer size is different. Therefore, EglAndroidImageCrop is not set in this case.
Assignee

Comment 38

4 years ago
attachment 8633356 [details] seems to have wired parts. OMXCodec emits the following log. It says allocated buffer size is (192 x 144), but actual video size is (176 x 144). The actual video size seems not delivered to GrallocTextureHostOGL. 

> I/OMXCodec( 3755): [OMX.ittiam.video.decoder.mpeg4] video dimensions are 192 x 144
> I/OMXCodec( 3755): [OMX.ittiam.video.decoder.mpeg4] Crop rect is 176 x 144 @ (0, 0)

From the following log, GrallocTextureHostOGL thought, video buffer size was (192 x 144).

> GrallocTextureHostOGL::PrepareTextureSource() 2 mDescriptorSize(192, 144) mSize(192, 144)
Assignee

Comment 39

4 years ago
On flame-kk, GrallocTextureHostOGL thought that valid video buffer size was (176 x 144) correctly.
Assignee

Comment 40

4 years ago
From comment 38, somehow correct valid video size seemed not delivered on the codeaurora's device.
Assignee

Comment 41

4 years ago
I recognize one problem. Since Bug 850566 fix, NewSurfaceDescriptorGralloc.size delivered valid video size. But it does not work since new gfx layer architecture. Then GrallocTextureHostOGL::mDescriptorSize also do not work correctly. It just notify gralloc buffer size now.

Valid video size info is delivered by ImageHost::SetPictureRect() on b2g v2.2.
Assignee

Comment 42

4 years ago
Posted patch log patch for b2g v2.2 (obsolete) — Splinter Review
Attachment #8632817 - Attachment is obsolete: true
Assignee

Comment 43

4 years ago
Yogendra, I crated a new log patch(attachment 8633565 [details] [diff] [review]). Can you get a logcat log by applying it and also check if the problem still happens?
Flags: needinfo?(yogendra)

Comment 44

4 years ago
Flags: needinfo?(yogendra)

Comment 45

4 years ago
When tested by applying the patch in Comment 42 ( attachment 8633565 [details] [diff] [review] ), issue is fixed. Now green line in is not seen.
 
The attached  logs are taken with rtsp://203.35.158.135/Brock_25k.3gp

I have also verified the issue with the following rtsp urls and issue is not seen.

rtsp://78.109.84.47/public/VideoPlayer-3GPPstreaming/3-Codecs_from_3G/AMR-12_MPEG4-74_QCIF_UMTS.3gp
rtsp://78.109.84.47/public/VideoPlayer-3GPPstreaming/3-Codecs_from_3G/AAC-16_MPEG4-84_QCIF_UMTS.mp4
rtsp://203.35.158.135/TRL/trl-upload/Highbit/brock_108k.3gp
rtsp://203.35.158.135/TRL/trl-upload/Highbit/mpeg4_aac_128k.3gp
Assignee

Comment 46

4 years ago
(In reply to Yogendra from comment #45)
> When tested by applying the patch in Comment 42 ( attachment 8633565 [details] [diff] [review]
> [details] [diff] [review] ), issue is fixed. Now green line in is not seen.

Thanks. It's great!
Assignee

Comment 48

4 years ago
Attachment #8625605 - Attachment is obsolete: true
Attachment #8625723 - Attachment is obsolete: true
Attachment #8633565 - Attachment is obsolete: true
Attachment #8625605 - Flags: feedback?(dwilson)
Assignee

Comment 49

4 years ago
attachment 8634130 [details] [diff] [review] does not update GLConsts.h by using GLParseRegistryXML.py since https://cvs.khronos.org/svn/repos/ogl/trunk/doc/registry/public/api/egl.xml does not have the following definitions.

> EGL_IMAGE_CROP_LEFT_ANDROID               0x3148
> EGL_IMAGE_CROP_TOP_ANDROID                0x3149
> EGL_IMAGE_CROP_RIGHT_ANDROID              0x314A
> EGL_IMAGE_CROP_BOTTOM_ANDROID             0x314B
Assignee

Comment 50

4 years ago
On quirk about EGL_ANDROID_image_crop is that an origin of the crop needs to be (crop.left == 0 && crop.top == 0). It is written on the following code.
 http://androidxref.com/5.1.1_r6/xref/frameworks/native/libs/gui/GLConsumer.cpp#1127
Assignee

Updated

4 years ago
Attachment #8634108 - Flags: review?(nical.bugzilla)
Assignee

Updated

4 years ago
Attachment #8634130 - Flags: review?(nical.bugzilla)
Attachment #8634130 - Flags: review?(jgilbert)
Attachment #8634108 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 8634130 [details] [diff] [review]
patch part 2 - Add EGL_ANDROID_image_crop support

Review of attachment 8634130 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/composite/ImageHost.cpp
@@ +94,5 @@
>      if (!img.mTextureSource && !mImages.IsEmpty()) {
>        img.mTextureSource = mImages.LastElement().mTextureSource;
>        mImages.RemoveElementAt(mImages.Length() - 1);
>      }
> +    img.mFrontBuffer->SetCropRect(img.mPictureRect);

I feel a bit uneasy about this, because it will only affect GrallocTextureHost but this code looks like something not-so-platform-specific that all texture types would implement.
Please add a comment here saying that some texture types don't implement SetCropRect and that it will have no effect in their case.
Attachment #8634130 - Flags: review?(nical.bugzilla) → review+
Assignee

Comment 54

4 years ago
Comment on attachment 8634159 [details] [diff] [review]
patch part 2 for b2gv2.2 - Add EGL_ANDROID_image_crop support

nical, can you review also a patch for v2.2. In the patch, ImageHost and ImageClient changes are a bit different.
Attachment #8634159 - Flags: review?(nical.bugzilla)
Assignee

Comment 55

4 years ago
Correct patch.
Attachment #8634159 - Attachment is obsolete: true
Attachment #8634159 - Flags: review?(nical.bugzilla)
Assignee

Updated

4 years ago
Attachment #8634191 - Flags: review?(nical.bugzilla)
Attachment #8634191 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 8634130 [details] [diff] [review]
patch part 2 - Add EGL_ANDROID_image_crop support

Review of attachment 8634130 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/opengl/EGLImageHelpers.cpp
@@ +19,5 @@
>  {
>      EGLint attrs[] = {
>          LOCAL_EGL_IMAGE_PRESERVED, LOCAL_EGL_TRUE,
> +        LOCAL_EGL_IMAGE_CROP_LEFT_ANDROID,    0,
> +        LOCAL_EGL_IMAGE_CROP_TOP_ANDROID,     0,

Ugh, origin top-left? Super gross. "Thanks", Android. :(

@@ +30,5 @@
> +        // No crop rect to set, so terminate the attrib array before the crop.
> +        attrs[2] = LOCAL_EGL_NONE;
> +    } else if (!sEGLLibrary.IsExtensionSupported(GLLibraryEGL::EGL_ANDROID_image_crop)) {
> +        // No EGL_ANDROID_image_crop support, so terminate the attrib array before the crop.
> +        attrs[2] = LOCAL_EGL_NONE;

This is trickier than it needs to be.

EGLint attrs[] = {
    LOCAL_EGL_IMAGE_PRESERVED, LOCAL_EGL_TRUE,
    LOCAL_EGL_NONE, LOCAL_EGL_NONE,
};

EGLint cropAttrs[] = {
    LOCAL_EGL_IMAGE_PRESERVED, LOCAL_EGL_TRUE,
    LOCAL_EGL_IMAGE_CROP_LEFT_ANDROID, 0,
    LOCAL_EGL_IMAGE_CROP_TOP_ANDROID, 0,
    LOCAL_EGL_IMAGE_CROP_RIGHT_ANDROID, aCropSize.width,
    LOCAL_EGL_IMAGE_CROP_BOTTOM_ANDROID, aCropSize.height,
    LOCAL_EGL_NONE, LOCAL_EGL_NONE,
};

bool hasCropRect = (aCropSize.width != 0 && aCropSize.height != 0);
EGLint* usedAttrs = attrs;
if (hasCropRect && sEGLLibrary.IsExtensionSupported(GLLibraryEGL::EGL_ANDROID_image_crop) {
    userAttrs = cropAttrs;
}

`attrs` needs to end with two `LOCAL_EGL_NONE`s, since we have found drivers which choke otherwise.

::: gfx/layers/opengl/GrallocTextureHost.cpp
@@ +344,5 @@
>      return;
>    }
>  
>    if (mEGLImage == EGL_NO_IMAGE) {
> +    gfx::IntSize cropSize;

Explicit initialization is better than implicit.

@@ +346,5 @@
>  
>    if (mEGLImage == EGL_NO_IMAGE) {
> +    gfx::IntSize cropSize;
> +    if (mCropSize.width < mSize.width ||
> +        mCropSize.height < mSize.height) {

Don't we already have this assurance from SetCropRect below?
Attachment #8634130 - Flags: review?(jgilbert) → review+
Assignee

Comment 57

4 years ago
Apply comments. Carry r=nical,jgilbert
Attachment #8634130 - Attachment is obsolete: true
Attachment #8635288 - Flags: review+
Assignee

Comment 58

4 years ago
Attachment #8634191 - Attachment is obsolete: true
Attachment #8635290 - Flags: review+
Assignee

Updated

4 years ago
Attachment #8609679 - Attachment is obsolete: true
Assignee

Comment 59

4 years ago
rebased.
Attachment #8635288 - Attachment is obsolete: true
Attachment #8635298 - Flags: review+
Assignee

Updated

4 years ago
Attachment #8634149 - Flags: review+
Assignee

Comment 60

4 years ago
Fix build failure.
Attachment #8635298 - Attachment is obsolete: true
Attachment #8635301 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/8438d2841dd6
https://hg.mozilla.org/mozilla-central/rev/dc6e872e521d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Assignee

Updated

4 years ago
Assignee

Comment 64

4 years ago
Yogendra, can you check if attachment 8635290 [details] [diff] [review] and attachment 8634149 [details] [diff] [review] work to the problem? They are candidate fix for b2g v2.2.
Flags: needinfo?(yogendra)

Comment 65

4 years ago
Flags: needinfo?(yogendra)

Comment 66

4 years ago
Verified the issue by applying patches in Comment 64 (8635290,8634149) and issue is fixed. No green line observed while playing the rtsp videos.
Assignee

Comment 67

4 years ago
Good. Thank you for the comfirmation!
Assignee

Comment 68

4 years ago
Comment on attachment 8634149 [details] [diff] [review]
patch part 1 for b2g v2.2 - Remove size from NewSurfaceDescriptorGralloc

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 #): bug 907745
User impact if declined: green line might appear during video playback on some hardwares
Testing completed: locally and codeaurora tested
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #8634149 - Flags: approval-mozilla-b2g37?
Assignee

Comment 69

4 years ago
Comment on attachment 8635290 [details] [diff] [review]
patch part 2 for b2gv2.2 - Add EGL_ANDROID_image_crop support

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 #): bug 907745
User impact if declined: green line might appear during video playback on some hardwares
Testing completed: locally and codeaurora tested
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #8635290 - Flags: approval-mozilla-b2g37?
Assignee

Updated

4 years ago
Attachment #8634149 - Flags: approval-mozilla-b2g37?
Assignee

Comment 70

4 years ago
Comment on attachment 8634149 [details] [diff] [review]
patch part 1 for b2g v2.2 - Remove size from NewSurfaceDescriptorGralloc

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 #): none
User impact if declined: green line might appear during video playback on some hardwares
Testing completed: locally and codeaurora tested
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #8634149 - Flags: approval-mozilla-b2g37?
Assignee

Updated

4 years ago
Attachment #8635290 - Flags: approval-mozilla-b2g37?
Assignee

Comment 71

4 years ago
Comment on attachment 8635290 [details] [diff] [review]
patch part 2 for b2gv2.2 - Add EGL_ANDROID_image_crop support

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
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 #): none
User impact if declined: green line might appear during video playback on some hardwares
Testing completed: locally and codeaurora tested
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #8635290 - Flags: approval-mozilla-b2g37?

Updated

4 years ago
blocking-b2g: 2.2? → 2.2+

Updated

4 years ago
Attachment #8634149 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Comment hidden (obsolete)

Updated

4 years ago
Flags: needinfo?(fan.luo)
Keywords: verifyme

Updated

4 years ago
Keywords: verifyme
Debug-only bustage it appears.
Assignee

Comment 76

4 years ago
Fix build error on debug build.
Attachment #8635290 - Attachment is obsolete: true
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 77

4 years ago
:RyanVM, can you try uplift to mozilla-b2g37_v2_2 again? attachment 8637914 [details] [diff] [review] fixed build failure on debug build.
Flags: needinfo?(ryanvm)
Will do.
Flags: needinfo?(ryanvm)
Assignee

Comment 81

4 years ago
(In reply to Wes Kocher (:KWierso) from comment #80)
> I had to back this out from b2g37 for various mochitest crashes:
> https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/4cff5605633b
> 
> https://treeherder.mozilla.org/logviewer.html#?job_id=160859&repo=mozilla-
> b2g37_v2_2

It seems to hit assertion checks. on b2g v2.2 Layer seems to work differently than master. Need to update a patch to handle the case.
Assignee

Comment 82

4 years ago
Fix test failures on b2g v2.2.
Attachment #8637914 - Attachment is obsolete: true
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Updated

4 years ago
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 84

4 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #83)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa8ab4d58c7d

The failures seems to be fixed.
You need to log in before you can comment on or make changes to this bug.