Bug 223850

Summary: REGRESSION(r268176): [GStreamer] media/video-orientation-canvas.html fails
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: PlatformAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, cgarcia, clopez, eric.carlson, ews-watchlist, glenn, gustavo, jer.noble, lmoura, menard, peng.liu6, philipj, sergio, vjaquez, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=95299
https://bugs.webkit.org/show_bug.cgi?id=224064
Attachments:
Description Flags
Patch clopez: review+

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