[Webkit-unassigned] [Bug 51407] A check should be in place to ensure that a media engine error has not occurred after MediaPlayer::play() is called

[bugzilla-daemon at webkit.org]

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


Eric Carlson <eric.carlson at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |eric.carlson at apple.com




--- Comment #6 from Eric Carlson <eric.carlson at apple.com>  2011-01-13 13:34:57 PST ---
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 77130 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=77130&action=review
> > 
> > > WebCore/html/HTMLMediaElement.cpp:2062
> > > -        startPlaybackProgressTimer();
> > > -        m_playing = true;
> > > +        // Ensure that we don't have any errors (especially that the media engine didn't give us an error).
> > > +        if (!m_error) {
> > > +            startPlaybackProgressTimer();
> > > +            m_playing = true;
> > > +        } else {
> > > +            // In the event of an error, ensure that the playback rate is 0.
> > > +            m_player->setRate(0);
> > > +        }
> > 
> > This assumes that an error triggered by calling m_player->play() will always happen synchronously, eg. that it will happen before the call returns, which is *definitely* not the case on all platforms.
> 
> This does not assume that the error setting will *always* happen synchronously, just that it *may* happen synchronously. In the event that the error triggered happens asynchronously instead (as is the assumption on most platforms it seems), then this should result in no change to the program's behavior because m_error will not have been set yet.
> 

I meant that the "fix" assumes the error will be set synchronously. If this behavior change is important, why not also do it when the error is fired asynchronously?

> So this leads me to the question: Is there a critical reason why we would never want to set the error synchronously here?

I am not aware of any reason in this case. The QuickTime based media engines don't end up setting it synchronously because we don't make calls to WebCore directly from framework callbacks so we don't end up recursing back into the framework.

Why is this necessary at all? The media engine signals the error in the first place, why doesn't it know that playback failed?

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