Bug 30002

Summary: [GStreamer] MediaPlayerPrivate::cancelLoad() can be implemented by setting state to NULL
Product: WebKit Reporter: Sebastian Dröge (slomo) <slomo>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, pnormand
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
proposed patch none

Description Sebastian Dröge (slomo) 2009-10-02 01:41:45 PDT
Hi,
MediaPlayerPrivate::cancelLoad() can be implemented by setting the state of playbin2 to NULL. This can be called at any time and will stop everything.
Comment 1 Philippe Normand 2009-10-14 07:22:10 PDT
Created attachment 41160 [details]
proposed patch
Comment 2 Gustavo Noronha (kov) 2009-10-15 06:14:09 PDT
Comment on attachment 41160 [details]
proposed patch

>  void MediaPlayerPrivate::cancelLoad()
>  {
> -    notImplemented();
> +    if (m_networkState < MediaPlayer::Loading || m_networkState == MediaPlayer::Loaded)
> +        return;
> +
> +    if (m_playBin)
> +        gst_element_set_state(m_playBin, GST_STATE_NULL);
>  }

Both Mac and Win call updateStates() here after doing their media library magics; do you know if there is a reason why we don't need it?
Comment 3 Philippe Normand 2009-10-15 06:42:19 PDT
(In reply to comment #2)
> (From update of attachment 41160 [details])
> >  void MediaPlayerPrivate::cancelLoad()
> >  {
> > -    notImplemented();
> > +    if (m_networkState < MediaPlayer::Loading || m_networkState == MediaPlayer::Loaded)
> > +        return;
> > +
> > +    if (m_playBin)
> > +        gst_element_set_state(m_playBin, GST_STATE_NULL);
> >  }
> 
> Both Mac and Win call updateStates() here after doing their media library
> magics; do you know if there is a reason why we don't need it?

Hmm right. updateStates() is called when we receive the STATE_CHANGE message but we don't handle STATE_NULL there. Will send a new patch, thanks for spotting that ;)
Comment 4 Gustavo Noronha (kov) 2009-10-15 07:15:12 PDT
Comment on attachment 41160 [details]
proposed patch

>  void MediaPlayerPrivate::cancelLoad()
>  {
> -    notImplemented();
> +    if (m_networkState < MediaPlayer::Loading || m_networkState == MediaPlayer::Loaded)
> +        return;
> +
> +    if (m_playBin)
> +        gst_element_set_state(m_playBin, GST_STATE_NULL);
>  }

According to further investigation by Philippe, updateStates is always called on element state transitions, so it is being done here.
Comment 5 WebKit Commit Bot 2009-10-15 07:32:00 PDT
Comment on attachment 41160 [details]
proposed patch

Clearing flags on attachment: 41160

Committed r49624: <http://trac.webkit.org/changeset/49624>
Comment 6 WebKit Commit Bot 2009-10-15 07:32:03 PDT
All reviewed patches have been landed.  Closing bug.