Bug 174241 - [GTK] Layout test media/video-inactive-playback.html is timing out
Summary: [GTK] Layout test media/video-inactive-playback.html is timing out
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 162582
  Show dependency treegraph
 
Reported: 2017-07-06 22:31 PDT by Michael Catanzaro
Modified: 2018-01-15 07:32 PST (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2017-07-06 22:31:54 PDT
Layout test media/video-inactive-playback.html is timing out on the GTK port since it was added in r218417 "[iOS] Do not pause playing video when application resigns active state." Updating expectations accordingly.
Comment 1 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-08-07 03:59:59 PDT
I looked into this and the problem is that the test sets restrictions on the "videoaudio" session:

        run('internals.setMediaSessionRestrictions("videoaudio", "")');
        run('internals.setMediaSessionRestrictions("videoaudio", "inactiveprocessplaybackrestricted")');

while forEachSession only finds a "video" session. This means nothing special happens and neither the expected "pause" or the expected "playing" event are fired.

(However, a "playing" event is fired as the result of the video finishing naturally.)

It's not clear to me why this test works at all on other platforms.

Jer, thoughts?
Comment 2 Jer Noble 2017-08-07 08:57:46 PDT
(In reply to Ms2ger from comment #1)
> I looked into this and the problem is that the test sets restrictions on the
> "videoaudio" session:
> 
>         run('internals.setMediaSessionRestrictions("videoaudio", "")');
>         run('internals.setMediaSessionRestrictions("videoaudio",
> "inactiveprocessplaybackrestricted")');
> 
> while forEachSession only finds a "video" session. This means nothing
> special happens and neither the expected "pause" or the expected "playing"
> event are fired.
> 
> (However, a "playing" event is fired as the result of the video finishing
> naturally.)
> 
> It's not clear to me why this test works at all on other platforms.
> 
> Jer, thoughts?

The file LayoutTests/media/content/test.{mp4,ogv} should have both an audio and a video track, so the resulting session should be of MediaType "VideoAudio", and not just "Video".  It sounds like that's the source of your problem.

If you look at HTMLMediaElement::mediaType(), it will return VideoAudio if the readyState >= HAVE_METADATA, the MediaPlayerPrivate return true for hasVideo() & hasAudio(), and the element is not muted.  So either this test is being run before the "loadedmetadata" event is fired, or the GTK media player doesn't think the file has audio.
Comment 3 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-08-10 00:47:43 PDT
The problem appears to be that Page::m_mutedState is AudioIsMuted. I have not yet been able to figure out how it got that way.
Comment 4 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-08-10 07:59:11 PDT
Looks like this and a bunch of other tests were broken by bug 162582.
Comment 5 Jer Noble 2017-08-10 09:07:56 PDT
(In reply to Ms2ger from comment #4)
> Looks like this and a bunch of other tests were broken by bug 162582.

Sounds like the GTK media player is reporting !hasAudio() when the page state is muted. That seems wrong.
Comment 6 Michael Catanzaro 2017-08-10 10:25:40 PDT
(In reply to Jer Noble from comment #5)
> (In reply to Ms2ger from comment #4)
> > Looks like this and a bunch of other tests were broken by bug 162582.
> 
> Sounds like the GTK media player is reporting !hasAudio() when the page
> state is muted. That seems wrong.

Charlie, this would be a good one for you or Calvaris to look into.
Comment 7 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-08-10 23:44:51 PDT
The code isn't specific to GTK, though:

bool HTMLMediaElement::effectiveMuted() const
{
    return muted() || (document().page() && document().page()->isAudioMuted());
}
Comment 8 Philippe Normand 2018-01-11 02:22:36 PST
Ms2ger was right in comment 3, the page is muted by default by WKTR so the test should un-mute it in its go function with internals.setPageMuted("");

I'm also a bit confused as to why this passes on other platforms, I'll try to dig this down further...
Comment 9 Philippe Normand 2018-01-11 03:20:16 PST
The current explanation I have is that in the player the volume element gets muted based on MediaPlayer::muted() value, which is true because the page is muted. I think this behavior is different on other platforms, so that's why they don't need to explicitely un-mute the page.
Comment 10 Philippe Normand 2018-01-11 03:32:19 PST
Another potential issue is that hasAudio() might return false before the gst pipeline is fully built and the audio track was discovered. This effectively short-cuts the page state too...
Comment 11 Philippe Normand 2018-01-15 03:15:55 PST
On mac the test passes because the AVF player doesn't support muting, so the volume is set to 0 from the player POV in HTMLMediaElement::updatePlayState() but the HTMLMediaElement m_muted attribute remains false. This is an inconsistency :(

Moreover the AVF player ::setMuted() method is implemented for both mac and iOS but supportsMuting() returns true only for iOS, yet another inconsistency.

If I replace the m_player->setMuted() call with a setMuted() call in HTMLMediaElement::updatePlayState() I get the test timeout on mac, same as in GTK.
Comment 12 Philippe Normand 2018-01-15 07:32:56 PST
Fixed in https://trac.webkit.org/r226948

But maybe the Apple folks should consider checking the little inconsistencies I found in the AVF player?