[Webkit-unassigned] [Bug 89122] [GStreamer] Audio device not closed after playing sound

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=89122





--- Comment #23 from Philippe Normand <pnormand at igalia.com>  2012-08-29 02:35:56 PST ---
(In reply to comment #22)
> (From update of attachment 160122 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=160122&action=review
> 
> This patch looks good to me, though I think that someone like Eric should look at the platform-independent changes.
> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:343
> > +        // Position queries don't work on a null pipeline and when we're at the end of the stream the pipeline is null.
> > +        if (m_seeking)
> > +            return m_seekTime;
> > +        if (m_mediaDuration)
> > +            return m_mediaDuration;
> > +    }
> 
> Hrm. I guess if the sync can handle position queries gracefully, then the comment is inaccurate?
> 

Yes... I'll elaborate the comment a bit more.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:497
> > +    bool prerollRequired = m_isEndReached && !m_player->mediaPlayerClient()->mediaPlayerLoop();
> > +    if (prerollRequired) {
> > +        LOG_MEDIA_MESSAGE("Seek on EOS, pre-rolling pipeline again.");
> > +        gst_element_set_state(m_playBin, GST_STATE_PAUSED);
> > +    }
> > +
> > +    // Avoid useless seeking.
> > +    if (time == playbackPosition()) {
> > +        if (prerollRequired)
> > +            gst_element_set_state(m_playBin, GST_STATE_NULL);
> > +        return;
> > +    }
> > +
> 
> Is it important to go from PAUSED back to NULL for no-op seeks here? If not, why can't you just do:
> 
> // Avoid useless seeking.
> if (time == playbackPosition())
>     return;
> 
> 
> if (m_isEndReached && !m_player->mediaPlayerClient()->mediaPlayerLoop()) {
>     LOG_MEDIA_MESSAGE("Seek on EOS, pre-rolling pipeline again.");
>     gst_element_set_state(m_playBin, GST_STATE_PAUSED);
> }


Well, now that I think about again, if pre-roll is required it means EOS was reached so we should use currentTime() directly instead of playbackPosition(). currentTime() gracefully handles the EOS case.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.