REGRESSION(r268176): [GStreamer] media/video-orientation-canvas.html fails

• Component: Platform
• Status: RESOLVED FIXED

[Philippe Normand]

--- /app/webkit/WebKitBuild/Release/layout-test-results/media/video-orientation-canvas-expected.txt
+++ /app/webkit/WebKitBuild/Release/layout-test-results/media/video-orientation-canvas-actual.txt
@@ -1,6 +1,6 @@

-EXPECTED (context.getImageData(88, 115, 1, 1).data == [208,0,0,255] with tolerance 5 OK
+EXPECTED (context.getImageData(88, 115, 1, 1).data == [208,0,0,255] with tolerance 5, OBSERVED '[0,0,0,255]' FAIL                                                                                                
 EXPECTED (context.getImageData(237, 88, 1, 1).data == [208,0,0,255] with tolerance 5 OK
-EXPECTED (context.getImageData(200, 237, 1, 1).data == [208,0,0,255] with tolerance 5 OK
+EXPECTED (context.getImageData(200, 237, 1, 1).data == [208,0,0,255] with tolerance 5, OBSERVED '[0,0,0,255]' FAIL                                                                                               
 END OF TEST

[Philippe Normand]

Created attachment 424493 [details]
Patch

[Peng Liu]

Comment on attachment 424493 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424493&action=review

> Source/WebCore/ChangeLog:9
> +        handling of the image rotation tags is now performed only the pipeline is not able to do it

Nit. s/only/only when/g.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2775
> +            m_shouldHandleOrientationTags = true;

Do we need to have a branch to set `m_shouldHandleOrientationTags` to false?

[Philippe Normand]

Comment on attachment 424493 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424493&action=review

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2775
>> +            m_shouldHandleOrientationTags = true;
> 
> Do we need to have a branch to set `m_shouldHandleOrientationTags` to false?

No, the variable is initialized to false already and only one pipeline is created per MediaPlayerPrivate.

[Carlos Alberto Lopez Perez]

*** Bug 224064 has been marked as a duplicate of this bug. ***

[Carlos Alberto Lopez Perez]

On r275376 I marked this test as failing, please remove the failed expectation once this lands.

[Carlos Alberto Lopez Perez]

BTW.. I think the regression was caused by r274076 and not by r268176 (as the title implies)

[Peng Liu]

(In reply to Carlos Alberto Lopez Perez from comment #6)
> BTW.. I think the regression was caused by r274076 and not by r268176 (as
> the title implies)

Oh? But r274076 is a change to Cocoa ports only.

[Philippe Normand]

(In reply to Carlos Alberto Lopez Perez from comment #6)
> BTW.. I think the regression was caused by r274076 and not by r268176 (as
> the title implies)

I found r268176 with results.webkit.org, though I admit I haven't tried a local revert to verify it.

[Carlos Alberto Lopez Perez]

(In reply to Peng Liu from comment #7)
> (In reply to Carlos Alberto Lopez Perez from comment #6)
> > BTW.. I think the regression was caused by r274076 and not by r268176 (as
> > the title implies)
> 
> Oh? But r274076 is a change to Cocoa ports only.

Ups.. yes.

You're right.

I got confused because r274076 showed as the first revision making this test fail on the bot result history.
But that was just because r274076 removed this test as expected failure from LayoutTests/platform/wk2/TestExpectations

So, ignore my comment. Sorry for the noise

[Carlos Alberto Lopez Perez]

Comment on attachment 424493 [details]
Patch

Patch looks fine to me.
I have tested it works and fixes the test.
Please remove the expected failure from GTK TestExpectations file when landing it.

[Philippe Normand]

Committed r275412 (236075@main): <https://commits.webkit.org/236075@main>

[Radar WebKit Bug Importer]

<rdar://problem/76142996>