Summary: | [GStreamer] Adjust internal size on http source element when receiving data if necessary | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andre Moreira Magalhaes <andrunko> | ||||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cgarcia, commit-queue, gustavo, menard, mrobinson, pnormand | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 115354 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Andre Moreira Magalhaes
2013-05-21 06:12:41 PDT
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. |