Bug 213947 – [GStreamer] media/vp9.html failing since check-in in r263894

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Lauro Moura]

Like the webrtc counterpart in bug213780, media/vp9.html is failing since added:

--- /home/buildbot/worker/gtk-linux-64-release-tests/build/layout-test-results/media/vp9-expected.txt
+++ /home/buildbot/worker/gtk-linux-64-release-tests/build/layout-test-results/media/vp9-actual.txt
@@ -1,5 +1,5 @@
 
 
-PASS HTMLMediaElement.canPlay VP9 
+FAIL HTMLMediaElement.canPlay VP9 assert_equals: canPlayType expected "maybe" but got "probably"
 PASS VP9 decoding

[Philippe Normand]

I don't think 2 bugs are needed.

*** This bug has been marked as a duplicate of bug 213780 ***

[Philippe Normand]

Oh it's 2 different tests. My bad :)

[Philippe Normand]

media/vp9.html has this assert:

assert_equals(video.canPlayType("video/mp4; codecs=vp9"), "maybe", "canPlayType");

Youenn, in GTK/WPE this returns "probably". Would it be OK to update the test changing the assert to a less restrictive test? Perhaps simply log the result in test expectation?

[Xabier Rodríguez Calvar]

(In reply to Philippe Normand from comment #3)
> media/vp9.html has this assert:
> 
> assert_equals(video.canPlayType("video/mp4; codecs=vp9"), "maybe",
> "canPlayType");
> 
> Youenn, in GTK/WPE this returns "probably". Would it be OK to update the
> test changing the assert to a less restrictive test? Perhaps simply log the
> result in test expectation?

From what I read in the spec at [1], it's clear that we're specifying the codec here so a user agent is per spec allowed to return probably.

[1] https://html.spec.whatwg.org/multipage/media.html#dom-navigator-canplaytype

This makes this test very highly dependent on the platform so I'd say we have to log the result and make different expectations for platforms. You can leave a general one with maybe and another glib one with probably.

[Philippe Normand]

Also exiting early if !window.internals.usingAppleInternalSDK is a bit sad, for non-Apple ports :(

[youenn fablet]

(In reply to Philippe Normand from comment #5)
> Also exiting early if !window.internals.usingAppleInternalSDK is a bit sad,
> for non-Apple ports :(
(In reply to Philippe Normand from comment #3)
> media/vp9.html has this assert:
> 
> assert_equals(video.canPlayType("video/mp4; codecs=vp9"), "maybe",
> "canPlayType");
> 
> Youenn, in GTK/WPE this returns "probably". Would it be OK to update the
> test changing the assert to a less restrictive test? Perhaps simply log the
> result in test expectation?

I think it is fine to check for either maybe or probably.

(In reply to Philippe Normand from comment #5)
> Also exiting early if !window.internals.usingAppleInternalSDK is a bit sad,
> for non-Apple ports :(

We could probably rewrite the test so that rejection of video.play() is tolerated in case of non Apple Internal SDK.

[Xabier Rodríguez Calvar]

Created attachment 405278 [details]
Patch

[Adrian Perez]

Comment on attachment 405278 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        * media/vp9.html: Non Apple ports return propably instead of ""

Typo: propably → probably

[Xabier Rodríguez Calvar]

Created attachment 405285 [details]
Patch

Add isApple which gives better granularity and allows GStreamer to run the second test.

[Darin Adler]

Comment on attachment 405285 [details]
Patch

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

> Source/WebCore/testing/Internals.cpp:5531
> +#if PLATFORM(MAC) || PLATFORM(IOS) || PLATFORM(MACCATALYST)
> +    return true;
> +#else
> +    return false;
> +#endif

This is an incorrect implementation. Will return false for watchOS and tvOS. Simpler is to just write PLATFORM(COCOA).

Not thrilled with the name of this. It seems imprecise. It will return false if you are running on Apple hardware but with a non-Apple port of WebKIt, and will return false if you are running on Windows with an Apple port of WebKit.

[Xabier Rodríguez Calvar]

(In reply to Darin Adler from comment #10)
> Comment on attachment 405285 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=405285&action=review
> 
> > Source/WebCore/testing/Internals.cpp:5531
> > +#if PLATFORM(MAC) || PLATFORM(IOS) || PLATFORM(MACCATALYST)
> > +    return true;
> > +#else
> > +    return false;
> > +#endif
> 
> This is an incorrect implementation. Will return false for watchOS and tvOS.
> Simpler is to just write PLATFORM(COCOA).

Ok,
 
> Not thrilled with the name of this. It seems imprecise. It will return false
> if you are running on Apple hardware but with a non-Apple port of WebKIt,
> and will return false if you are running on Windows with an Apple port of
> WebKit.

I'll name it isApplePort.

[Xabier Rodríguez Calvar]

Created attachment 405344 [details]
Patch

Implemented with internals.usingGStreamer()

[Xabier Rodríguez Calvar]

Created attachment 405350 [details]
Patch for landing

[EWS]

Committed r264975: <https://trac.webkit.org/changeset/264975>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405350 [details].

[Radar WebKit Bug Importer]

<rdar://problem/66212906>