Follow up on bug #115354.
Created attachment 202423 [details] Adjust internal size when receiving data if needed This patch replaces https://bugs.webkit.org/attachment.cgi?id=202290 from bug #115354
Attachment 202423 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp']" exit_code: 1 Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:917: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 202424 [details] Adjust internal size when receiving data if needed Updated Changelog to use proper bug number.
Attachment 202424 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp']" exit_code: 1 Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:917: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 202424 [details] Adjust internal size when receiving data if needed 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. > 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) > 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.
Created attachment 203946 [details] Adjust internal size when receiving data if needed (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
Attachment 203946 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp']" exit_code: 1 Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:1015: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #6) > Created an attachment (id=203946) [details] > Adjust internal size when receiving data if needed > > (In reply to comment #5) > > (From update of attachment 202424 [details] [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 You are right, I read the comparison wrongly. But still, you should do: if (priv->size && priv->offset > priv->size) or if (priv->size > 0 && priv->offset > priv->size) to make the style bot happy.
Created attachment 203947 [details] Adjust internal size when receiving data if needed (In reply to comment #8) > > > > 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 > > You are right, I read the comparison wrongly. But still, you should do: > > if (priv->size && priv->offset > priv->size) > > or > > if (priv->size > 0 && priv->offset > priv->size) > > to make the style bot happy. hehe, done :).
Comment on attachment 203947 [details] Adjust internal size when receiving data if needed Clearing flags on attachment: 203947 Committed r151731: <http://trac.webkit.org/changeset/151731>
All reviewed patches have been landed. Closing bug.