1167799 - MPEG4 videos of some resolutions display with a green bar

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Diego Wilson [:diego]]

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.

Comment 1

[Diego Wilson [:diego]]

Attachment: Snap buffer scaling to integer pixels

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

Comment 2

[Sotaro Ikeda [:sotaro]]

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.

Comment 3

[Diego Wilson [:diego]]

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.

Comment 4

[Matt Woodrow (:mattwoodrow)]

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.

Comment 5

[Bas Schouten (:bas.schouten)]

(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.

Comment 6

[Diego Wilson [:diego]]

(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?

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

Sotaro, this has as potential of becoming 2.2+, and QC is tracking it, can you follow up when you have a chance?

Comment 8

[Sotaro Ikeda [:sotaro]]

Okey, I take a look.

Comment 9

[Matt Woodrow (:mattwoodrow)]

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.

Comment 10

[Sotaro Ikeda [:sotaro]]

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.

Comment 11

[Sotaro Ikeda [:sotaro]]

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.

Comment 12

[Sotaro Ikeda [:sotaro]]

diego, can you provide a content and screenshot of the symptom?

Comment 13

[Sotaro Ikeda [:sotaro]]

(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?

Comment 14

[Sotaro Ikeda [:sotaro]]

(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.

Comment 15

[Diego Wilson [:diego]]

Attachment: Screenshot

Sotaro,

Please find a screenshot attached.

Comment 16

[Sotaro Ikeda [:sotaro]]

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.

Comment 17

[Sotaro Ikeda [:sotaro]]

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

Comment 18

[Sotaro Ikeda [:sotaro]]

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.

Comment 19

[Sotaro Ikeda [:sotaro]]

Diego, your device supports EGL_ANDROID_image_crop extension?

Comment 20

[Sotaro Ikeda [:sotaro]]

(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?

Comment 21

[Diego Wilson [:diego]]

(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.

Comment 22

[Sotaro Ikeda [:sotaro]]

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

Comment 23

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Necessary change to egl.xml

Comment 24

[Sotaro Ikeda [:sotaro]]

(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.

Comment 25

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Add EGL_ANDROID_image_crop support

Comment 26

[Sotaro Ikeda [:sotaro]]

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

diego, can you confirm if the patch works.

Comment 27

[Sotaro Ikeda [:sotaro]]

Attachment: patch for b2g v2.2 - Add EGL_ANDROID_image_crop support

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Any updates on this bug?

Comment 29

[Inder]

Ryan - apologies for the delay, we are verifying it and will update shortly.

Comment 30

[Yogendra]

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.

Comment 31

[Sotaro Ikeda [:sotaro]]

(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.

Comment 32

[Sotaro Ikeda [:sotaro]]

Attachment: log patch for b2g v2.2

Comment 33

[Sotaro Ikeda [:sotaro]]

Inder, Yogendra, can you taka a logcat log by applying attachment 8632817 [details] [diff] [review]?

Comment 34

[Yogendra]

Attachment: logs taken with build by appyling patch 8632817

Comment 35

[Yogendra]

logs are taken by playing rtsp link: rtsp://203.35.158.135/Brock_25k.3gp

Comment 36

[Sotaro Ikeda [:sotaro]]

(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.

Comment 37

[Sotaro Ikeda [:sotaro]]

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

Comment 38

[Sotaro Ikeda [:sotaro]]

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)

Comment 39

[Sotaro Ikeda [:sotaro]]

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

Comment 40

[Sotaro Ikeda [:sotaro]]

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

Comment 41

[Sotaro Ikeda [:sotaro]]

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.

Comment 42

[Sotaro Ikeda [:sotaro]]

Attachment: log patch for b2g v2.2

Comment 43

[Sotaro Ikeda [:sotaro]]

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?

Comment 44

[Yogendra]

Attachment: logcat logs taken by applying patch 8633565

Comment 45

[Yogendra]

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

Comment 46

[Sotaro Ikeda [:sotaro]]

(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!

Comment 47

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 - Remove size from NewSurfaceDescriptorGralloc

Comment 48

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Add EGL_ANDROID_image_crop support

Comment 49

[Sotaro Ikeda [:sotaro]]

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

Comment 50

[Sotaro Ikeda [:sotaro]]

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

Comment 51

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 1 for b2g v2.2 - Remove size from NewSurfaceDescriptorGralloc

Comment 52

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 for b2gv2.2 - Add EGL_ANDROID_image_crop support

Comment 53

[Nicolas Silva [:nical]]

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.

Comment 54

[Sotaro Ikeda [:sotaro]]

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.

Comment 55

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 for b2gv2.2 - Add EGL_ANDROID_image_crop support

Correct patch.

Comment 56

[Kelsey Gilbert [:jgilbert]]

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?

Comment 57

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Add EGL_ANDROID_image_crop support

Apply comments. Carry r=nical,jgilbert

Comment 58

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 for b2gv2.2 - Add EGL_ANDROID_image_crop support

Comment 59

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Add EGL_ANDROID_image_crop support

rebased.

Comment 60

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 - Add EGL_ANDROID_image_crop support

Fix build failure.

Comment 61

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c37461312307

Comment 62

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8438d2841dd6
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc6e872e521d

Comment 63

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8438d2841dd6
https://hg.mozilla.org/mozilla-central/rev/dc6e872e521d

Comment 64

[Sotaro Ikeda [:sotaro]]

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.

Comment 65

[Yogendra]

Attachment: logs taken by applying patches 8634149 and 8635290

Comment 66

[Yogendra]

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

Comment 67

[Sotaro Ikeda [:sotaro]]

Good. Thank you for the comfirmation!

Comment 68

[Sotaro Ikeda [:sotaro]]

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

Comment 69

[Sotaro Ikeda [:sotaro]]

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

Comment 70

[Sotaro Ikeda [:sotaro]]

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

Comment 71

[Sotaro Ikeda [:sotaro]]

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

Comment 72

[Josh Cheng [:josh]]

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

Hi Norry, Please help to verify when patch landed on 2.2 Thanks!

Comment 73

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7f99ab28df3b
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/4f4c37f64770

Comment 74

[Ryan VanderMeulen [:RyanVM]]

Backed out for bustage.
https://treeherder.mozilla.org/logviewer.html#?job_id=159980&repo=mozilla-b2g37_v2_2

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/d2fed1cc44dc

Comment 75

[Ryan VanderMeulen [:RyanVM]]

Debug-only bustage it appears.

Comment 76

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 for b2gv2.2 - Add EGL_ANDROID_image_crop support

Fix build error on debug build.

Comment 77

[Sotaro Ikeda [:sotaro]]

:RyanVM, can you try uplift to mozilla-b2g37_v2_2 again? attachment 8637914 [details] [diff] [review] fixed build failure on debug build.

Comment 78

[Ryan VanderMeulen [:RyanVM]]

Will do.

Comment 79

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5981fda08dea
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0ba9636b3854

Comment 80

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 81

[Sotaro Ikeda [:sotaro]]

(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.

Comment 82

[Sotaro Ikeda [:sotaro]]

Attachment: patch part 2 for b2gv2.2 - Add EGL_ANDROID_image_crop support

Fix test failures on b2g v2.2.

Comment 83

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa8ab4d58c7d

Comment 84

[Sotaro Ikeda [:sotaro]]

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

The failures seems to be fixed.

Comment 85

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ccd992ce3283
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/35c740883fd9

Comment 86

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/ccd992ce3283
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/35c740883fd9