Bug 223850 - REGRESSION(r268176): [GStreamer] media/video-orientation-canvas.html fails
Summary: REGRESSION(r268176): [GStreamer] media/video-orientation-canvas.html fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
: 224064 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-03-28 02:52 PDT by Philippe Normand
Modified: 2021-04-02 01:56 PDT (History)
15 users (show)

See Also:


Attachments
Patch (7.16 KB, patch)
2021-03-28 05:28 PDT, Philippe Normand
clopez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2021-03-28 02:52:32 PDT
--- /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
Comment 1 Philippe Normand 2021-03-28 05:28:28 PDT
Created attachment 424493 [details]
Patch
Comment 2 Peng Liu 2021-03-30 16:13:30 PDT
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?
Comment 3 Philippe Normand 2021-03-31 01:15:38 PDT
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.
Comment 4 Carlos Alberto Lopez Perez 2021-04-01 13:47:23 PDT
*** Bug 224064 has been marked as a duplicate of this bug. ***
Comment 5 Carlos Alberto Lopez Perez 2021-04-01 13:48:34 PDT
On r275376 I marked this test as failing, please remove the failed expectation once this lands.
Comment 6 Carlos Alberto Lopez Perez 2021-04-01 13:51:46 PDT
BTW.. I think the regression was caused by r274076 and not by r268176 (as the title implies)
Comment 7 Peng Liu 2021-04-01 13:58:50 PDT
(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.
Comment 8 Philippe Normand 2021-04-01 14:16:40 PDT
(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.
Comment 9 Carlos Alberto Lopez Perez 2021-04-01 14:17:48 PDT
(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
Comment 10 Carlos Alberto Lopez Perez 2021-04-01 14:23:06 PDT
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.
Comment 11 Philippe Normand 2021-04-02 01:55:45 PDT
Committed r275412 (236075@main): <https://commits.webkit.org/236075@main>
Comment 12 Radar WebKit Bug Importer 2021-04-02 01:56:15 PDT
<rdar://problem/76142996>