After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 742638 - mpegpsdemux: dead code
mpegpsdemux: dead code
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-01-09 11:15 UTC by Luis de Bethencourt
Modified: 2015-01-12 13:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
original behaviour (594 bytes, patch)
2015-01-09 11:15 UTC, Luis de Bethencourt
committed Details | Review
current behaviour (1.04 KB, patch)
2015-01-09 11:21 UTC, Luis de Bethencourt
rejected Details | Review

Description Luis de Bethencourt 2015-01-09 11:15:02 UTC
Created attachment 294149 [details] [review]
original behaviour

In gst/mpegdemux/gstmpegdemux.c there is a gboolean initialized to FALSE and then the only usage on found is in these two blocks a few lines below:

  while (found && fscr < scr) {
     [...]
  }

  while (found && fscr > scr && offset > 0) {
     [...]
  }
Comment 1 Luis de Bethencourt 2015-01-09 11:21:15 UTC
The two options are to following the original behavior [0] which was removed in the commit 7bcd991f93ea813c1b079606ab4640b6a3898742 [1].

Or keep the current behaviour which hasn't had any bugs reported since that commit 2 years ago and remove the gboolean and blocks altogether. Both options attached.

[0] http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst/mpegdemux/gstmpegdemux.c?id=8147669971151c33d12c645a242406e518291ff0#n1060
[1] http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=7bcd991f93ea813c1b079606ab4640b6a3898742
Comment 2 Luis de Bethencourt 2015-01-09 11:21:40 UTC
Created attachment 294151 [details] [review]
current behaviour
Comment 4 Tim-Philipp Müller 2015-01-12 09:50:50 UTC
Comment on attachment 294151 [details] [review]
current behaviour

Clearly there was an intention behind this code..
Comment 5 Tim-Philipp Müller 2015-01-12 09:53:16 UTC
Comment on attachment 294149 [details] [review]
original behaviour

This makes more sens to me, at least it should match the intention behind the code more closely. (There'll still be a lot of scanning if our original offset guesstimate was wrong, but that's unrelated to this change)
Comment 6 Tim-Philipp Müller 2015-01-12 09:53:58 UTC
And you can also remove the gboolean found = FALSE at the top then I believe.
Comment 7 Luis de Bethencourt 2015-01-12 13:04:16 UTC
I agree. Fix submitted.
Comment 8 Luis de Bethencourt 2015-01-12 13:04:43 UTC
Comment on attachment 294149 [details] [review]
original behaviour

Thanks Tim! :)