WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
116534
[GStreamer] Adjust internal size on http source element when receiving data if necessary
https://bugs.webkit.org/show_bug.cgi?id=116534
Summary
[GStreamer] Adjust internal size on http source element when receiving data i...
Andre Moreira Magalhaes
Reported
2013-05-21 06:12:41 PDT
Follow up on
bug #115354
.
Attachments
Adjust internal size when receiving data if needed
(2.44 KB, patch)
2013-05-21 06:15 PDT
,
Andre Moreira Magalhaes
no flags
Details
Formatted Diff
Diff
Adjust internal size when receiving data if needed
(2.44 KB, patch)
2013-05-21 06:19 PDT
,
Andre Moreira Magalhaes
pnormand
: review+
cgarcia
: commit-queue-
Details
Formatted Diff
Diff
Adjust internal size when receiving data if needed
(2.47 KB, patch)
2013-06-06 09:58 PDT
,
Andre Moreira Magalhaes
no flags
Details
Formatted Diff
Diff
Adjust internal size when receiving data if needed
(2.47 KB, patch)
2013-06-06 10:16 PDT
,
Andre Moreira Magalhaes
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Andre Moreira Magalhaes
Comment 1
2013-05-21 06:15:44 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
WebKit Commit Bot
Comment 2
2013-05-21 06:18:09 PDT
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.
Andre Moreira Magalhaes
Comment 3
2013-05-21 06:19:05 PDT
Created
attachment 202424
[details]
Adjust internal size when receiving data if needed Updated Changelog to use proper bug number.
WebKit Commit Bot
Comment 4
2013-05-21 06:21:12 PDT
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.
Carlos Garcia Campos
Comment 5
2013-06-06 06:07:23 PDT
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.
Andre Moreira Magalhaes
Comment 6
2013-06-06 09:58:43 PDT
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
WebKit Commit Bot
Comment 7
2013-06-06 10:00:06 PDT
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.
Carlos Garcia Campos
Comment 8
2013-06-06 10:05:42 PDT
(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.
Andre Moreira Magalhaes
Comment 9
2013-06-06 10:16:07 PDT
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 :).
WebKit Commit Bot
Comment 10
2013-06-19 06:23:22 PDT
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
>
WebKit Commit Bot
Comment 11
2013-06-19 06:23:25 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug