[Webkit-unassigned] [Bug 90084] [GStreamer] Live stream support is weak

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Philippe Normand <pnormand at igalia.com>  2012-07-07 20:02:13 PST ---
(In reply to comment #7)

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1448
> > +    if (attemptTotalBytesCache && cacheTotalBytes() && totalBytes() && !isLiveStream() && m_initialPreload == MediaPlayer::Auto) {
> > +        setPreload(m_initialPreload);
> > +        gst_element_set_state(m_playBin, GST_STATE_NULL);
> > +        gst_element_set_state(m_playBin, GST_STATE_PAUSED);
> 
> I'm not exactly sure why you want to only cache the total bytes sometimes...
> 

There's no need to cache it if durationChanged() is called during on-disk buffering, which can be done only if totalBytes() is non-null anyway.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:203
> > +            unsigned m_totalBytes;
> > +            bool m_totalBytesCached;
> 
> This should probably be called m_cachedTotalBytes, because at first I read it like "The total bytes we have cached" and the name I suggested is less ambiguous.
> 

Oh yes, I didn't realize that ambiguation indeed.

> Even better is if you could find a way to eliminate m_totalBytesCached entirely, perhaps by initializing  m_totalBytes to -1 (perhaps make it a long).
> 
> Assuming you don't do that...an unsigned seems an odd type to use for some number of bytes. Perhaps size_t would be better?

Hum, I'll give this some thoughts and testing. Thanks a lot for the initial feedback!

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