[webkit-reviews] review requested: [Bug 116534] [GStreamer] Adjust internal size on http source element when receiving data if necessary : [Attachment 203946] Adjust internal size when receiving data if needed

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 6 10:00:08 PDT 2013


Andre Moreira Magalhaes <andrunko at gmail.com> has asked	for review:
Bug 116534: [GStreamer] Adjust internal size on http source element when
receiving data if necessary
https://bugs.webkit.org/show_bug.cgi?id=116534

Attachment 203946: Adjust internal size when receiving data if needed 
https://bugs.webkit.org/attachment.cgi?id=203946&action=review

------- Additional Comments from Andre Moreira Magalhaes <andrunko at gmail.com>
(In reply to comment #5)
> (From update of attachment 202424 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=202424&action=review
> 
> Please, fix coding style issues before landing.
> 
> >
Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1019
> > +	 // priv->size == 0 if received length on didReceiveResponse < 0
> 
> Nit: According to the coding style comments should finish with a period.
> 
Done

> >
Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1020
> > +	 if (priv->size != 0 && priv->offset > priv->size) {
> 
> This is confusing offset and size are both unsigned so if offset == 0 it will
never be > size. This could probably just be 
> 
> if (priv->offset > priv->size)
> 
But IIRC, if priv->size == 0 (set in didReceiveResponse) we don't want/need to
update the internal size as didReceiveResponse will disable seek, etc on the
gst appsrc. No change here

> >
Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1021
> > +	     GST_DEBUG_OBJECT(m_src, "Updating internal size from %ld to %ld",
priv->size, priv->offset);
> 
> size and offset are guint64, you should probably use G_GUINT64_FORMAT instead
of %ld.
Done


More information about the webkit-reviews mailing list