Bug 138107 - Introduce a new abstract class called AudioProducer and keep a set of AudioProducers rather than the active MediaSessions in Document
Summary: Introduce a new abstract class called AudioProducer and keep a set of AudioPr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ada Chan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-27 15:39 PDT by Ada Chan
Modified: 2014-10-28 15:21 PDT (History)
13 users (show)

See Also:


Attachments
Patch (16.50 KB, patch)
2014-10-27 17:51 PDT, Ada Chan
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ada Chan 2014-10-27 15:39:54 PDT
In preparation for handling Web Audio and plug-ins for the isPlayingAudio() API in Document, introduce an abstract class called AudioProducer that HTMLMediaElement, PluginView, and AudioContext will implement.  Initially it'll have this method:

virtual bool isPlayingAudio() = 0;

Instead of keeping a set of MediaSessions in Document, we'll be keeping a set of AudioProducers instead.  The Document will query its AudioProducers for its overall audio playing state in Document::updateIsPlayingAudio().
Comment 1 Ada Chan 2014-10-27 17:51:06 PDT
Created attachment 240517 [details]
Patch
Comment 2 Eric Carlson 2014-10-28 11:29:01 PDT
Comment on attachment 240517 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:6028
> +bool HTMLMediaElement::isPlayingAudio()
> +{
> +    return m_mediaSession->hasMediaCharacteristics(MediaSession::MediaCharacteristicAudible) && m_mediaSession->state() == MediaSession::Playing;
> +}

Nit: why ask the session when it will just call back into HTMLMediaElement? Something like "return hasAudio() && isPlaying()" should be equivalent.
Comment 3 Ada Chan 2014-10-28 15:21:23 PDT
(In reply to comment #2)
> Comment on attachment 240517 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240517&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.cpp:6028
> > +bool HTMLMediaElement::isPlayingAudio()
> > +{
> > +    return m_mediaSession->hasMediaCharacteristics(MediaSession::MediaCharacteristicAudible) && m_mediaSession->state() == MediaSession::Playing;
> > +}
> 
> Nit: why ask the session when it will just call back into HTMLMediaElement?
> Something like "return hasAudio() && isPlaying()" should be equivalent.

I changed it to call hasAudio() directly.  I'm still querying m_mediaSession's state here for now because HTMLMediaElement::m_playing is not updated to the correct value yet when MediaSession::setState() is called (which triggers Document::updateIsPlayingAudio() to be called).

I've filed https://bugs.webkit.org/show_bug.cgi?id=138153.  That bug will cover removing Remove MediaSessionClient::hasMediaCharacteristics() and MediaSessionClient::mediaStateDidChange().  As part of that change, I'll call Document::updateIsPlayingAudio() after HTMLMediaElement::m_playing has been updated, and then I can change HTMLMediaElement::isPlayingAudio() to call isPlaying() directly.
Comment 4 Ada Chan 2014-10-28 15:21:56 PDT
Committed:
http://trac.webkit.org/changeset/175273