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

[Stefan Kost]

Wim Taymans schrieb:
> 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

I think Edward did a good job here. Compilers do indeed leave a lot of dead code
in. E.g. removing dummy _init() or _set/_get_property functions is nice, as it
makes the code smaller. I think we have enough plugin to copy'n'paste
boilerplate when we atuall need it.

One controversical thing is to remove dead assignments, like "ret=FALSE in some
branches" and dead assignments in return values. There was one change in core,
where it might have been better to turn the dead assignment of the return result
into an if + GST_WARNING.

Stefan

> 
> 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