[gst-devel] [gst-cvs] gst-plugins-ugly: mpegstream: Remove dead assignments.

Edward Hervey bilboed at gmail.com
Wed Apr 22 15:05:59 CEST 2009


On Wed, 2009-04-22 at 14:36 +0200, Wim Taymans wrote:
> Also I'm a bit annoyed with these cleanups. While the compiler has
> probably detected something that might not be optimal, it's also
> completely irrelevant. All relevant compilers will happily use their
> analysis to not compile the dead assignments at all. It's not the job
> of the programmer to perform this compiler optimisation step manually,
> IMO.

  gcc doesn't do it when I checked a few weeks back, unless you don't
consider it a relevent compiler.

> 
> I'm worried that all these cleanups make the code harder to read and
> in some cases even remove useful clues to what the author might have
> meant in case of a bug (like here?).

  I'm not removing those (there's a ton of dead assignments in various
demuxer's parsing code which I didn't/won't commit).

  What I'll do from now on for those is put big fat FIXME warnings
before each of those. Like that it at least won't go unnoticed and won't
bother people (apparently?).

   Edward

> 
> Wim
> 
> On Wed, Apr 22, 2009 at 12:58 PM, Edward Hervey <bilboed at gmail.com> wrote:
> > On Wed, 2009-04-22 at 11:27 +0200, Peter Kjellerstedt wrote:
> >> Could it be that someone had intended to clamp the value?
> >> I.e., something like this:
> >>
> >>     update_time = CLAMP (timestamp, mpeg_parse->current_segment.start,
> >>         mpeg_parse->current_segment.stop);
> >>
> >> I have not looked at the surrounding code so I do not know
> >> if it is the case, but it seems plausible.
> >
> >  Good point, maybe the maintainers could give use more input (although
> > I guess it won't matter *that* much considering we're no longer this
> > demuxer (rank:SECONDARY) but the one in -bad (rank:PRIMARY)).
> >
> >   Edward
> >
> > P.S. And I'm glad people actually read the commit messages in their
> > entirety :)
> >
> >>
> >> //Peter
> >>
> >> > -----Original Message-----
> >> > From: Edward Hervey [mailto:bilboed at kemper.freedesktop.org]
> >> > Sent: den 21 april 2009 20:41
> >> > To: gstreamer-cvs at lists.sourceforge.net
> >> > Subject: [gst-cvs] gst-plugins-ugly: mpegstream: Remove dead
> >> > assignments.
> >> >
> >> > Module: gst-plugins-ugly
> >> > Branch: master
> >> > Commit: df349f9359d0154ca44bc129e6062a61b68cfba3
> >> > URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-
> >> > ugly/commit/?id=df349f9359d0154ca44bc129e6062a61b68cfba3
> >> >
> >> > Author: Edward Hervey <bilboed at bilboed.com>
> >> > Date:   Tue Apr 21 20:20:02 2009 +0200
> >> >
> >> > mpegstream: Remove dead assignments.
> >> >
> >> > The duplicate assignment of update_time was weird... but it seems
> >> > normal that it's indeed the second statement which is the valid one.
> >> >
> >> > ---
> >> >
> >> >  gst/mpegstream/gstmpegdemux.c |    7 ++-----
> >> >  gst/mpegstream/gstmpegparse.c |    1 -
> >> >  2 files changed, 2 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/gst/mpegstream/gstmpegdemux.c
> >> > b/gst/mpegstream/gstmpegdemux.c
> >> > index b867807..c8d55d8 100644
> >> > --- a/gst/mpegstream/gstmpegdemux.c
> >> > +++ b/gst/mpegstream/gstmpegdemux.c
> >> > @@ -1072,7 +1070,6 @@ gst_mpeg_demux_send_subbuffer (GstMPEGDemux *
> >> > mpeg_demux,
> >> >      GstClockTimeDiff diff;
> >> >      guint64 update_time;
> >> >
> >> > -    update_time = MIN (timestamp, mpeg_parse->current_segment.stop);
> >> >      update_time = MAX (timestamp, mpeg_parse->current_segment.start);
> >> >      diff = GST_CLOCK_DIFF (mpeg_parse->current_segment.last_stop, update_time);
> >> >      if (diff > GST_SECOND * 2) {
> >>
> >>
> >> ------------------------------------------------------------------------------
> >> Stay on top of everything new and different, both inside and
> >> around Java (TM) technology - register by April 22, and save
> >> $200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
> >> 300 plus technical and hands-on sessions. Register today.
> >> Use priority code J9JMT32. http://p.sf.net/sfu/p
> >> _______________________________________________
> >> gstreamer-devel mailing list
> >> gstreamer-devel at lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
> >
> >
> > ------------------------------------------------------------------------------
> > Stay on top of everything new and different, both inside and
> > around Java (TM) technology - register by April 22, and save
> > $200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
> > 300 plus technical and hands-on sessions. Register today.
> > Use priority code J9JMT32. http://p.sf.net/sfu/p
> > _______________________________________________
> > gstreamer-devel mailing list
> > gstreamer-devel at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
> >
> 
> ------------------------------------------------------------------------------
> Stay on top of everything new and different, both inside and 
> around Java (TM) technology - register by April 22, and save
> $200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
> 300 plus technical and hands-on sessions. Register today. 
> Use priority code J9JMT32. http://p.sf.net/sfu/p
> _______________________________________________
> gstreamer-devel mailing list
> gstreamer-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/gstreamer-devel





More information about the gstreamer-devel mailing list