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

[Wim Taymans]

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.

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?).

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
>