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 736905 - videomixer: Removed unwanted instruction
videomixer: Removed unwanted instruction
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other Linux
: Normal minor
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-18 13:29 UTC by Sanjay NM
Modified: 2015-02-13 11:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
videomixer: Removed unwanted instruction (721 bytes, patch)
2014-09-18 13:29 UTC, Sanjay NM
needs-work Details | Review
videomixer: Removed redundant instruction (971 bytes, patch)
2014-09-26 12:05 UTC, Sanjay NM
needs-work Details | Review
videoaggregator: Removed unnecessary assignment (1.13 KB, patch)
2014-10-09 05:22 UTC, Sanjay NM
needs-work Details | Review
videoaggregator: Removed unwanted assignment to start_time (1.55 KB, patch)
2014-10-10 04:51 UTC, Sanjay NM
none Details | Review
videomixer: Removed unwanted assignment to start_time (1.41 KB, patch)
2014-10-10 05:09 UTC, Sanjay NM
none Details | Review

Description Sanjay NM 2014-09-18 13:29:35 UTC
Created attachment 286497 [details] [review]
videomixer: Removed unwanted instruction

Removed unwanted instruction
Comment 1 Thibault Saunier 2014-09-18 15:24:37 UTC
It would be nice if you could explain a bit better why we do not need that instruction in the commit message. Also it would be great if you could have a look at compositior/videoaggregator instead of videomixer as the goal is to replace the videomixer implementation by the compositor one during that 1.6 cycle.
Comment 2 Sebastian Dröge (slomo) 2014-09-23 20:18:03 UTC
Also please put the Bugzilla URL into the commit message as a last line
Comment 3 Sanjay NM 2014-09-26 11:40:13 UTC
Sorry I pulled code and getting compilation error. Will submit these changes on Monday
Comment 4 Sanjay NM 2014-09-26 12:05:41 UTC
Created attachment 287150 [details] [review]
videomixer: Removed redundant instruction

Updated the Patch as per the comments

start_time *= ABS (mix->segment.rate);

In function gst_videomixer2_sink_clip, after the above assignment start_time is nowhere used. So removed this instruction.
Comment 5 Sebastian Dröge (slomo) 2014-10-05 19:23:01 UTC
Comment on attachment 287150 [details] [review]
videomixer: Removed redundant instruction

Note that start_time is assigned and not really used above that too.

Did you check if the same change is relevant to compositor / videoaggregator too?
Comment 6 Sanjay NM 2014-10-09 05:22:48 UTC
Created attachment 288091 [details] [review]
videoaggregator: Removed unnecessary assignment

videoaggregator: Removed unnecessary assignment

Below instruction is not required as start_time is not used after this assignment

 start_time *= ABS (agg->segment.rate);
Comment 7 Sanjay NM 2014-10-09 05:24:57 UTC
start_time is used before this assignment in videomixer

  start_time = MAX (start_time, mixcol->collect.segment.start);
  start_time =
      gst_segment_to_running_time (&mixcol->collect.segment,
      GST_FORMAT_TIME, start_time);

Verified videoaggregator and have made changes in that too, have submitted a patch for the same
Comment 8 Sebastian Dröge (slomo) 2014-10-09 08:06:45 UTC
(In reply to comment #7)
> start_time is used before this assignment in videomixer
> 
>   start_time = MAX (start_time, mixcol->collect.segment.start);
>   start_time =
>       gst_segment_to_running_time (&mixcol->collect.segment,
>       GST_FORMAT_TIME, start_time);

It's only assigned but never really used... thus can go away
Comment 9 Sanjay NM 2014-10-10 04:51:25 UTC
Created attachment 288183 [details] [review]
videoaggregator: Removed unwanted assignment to start_time

Removed unwanted assignment to start_time as start_time is not being used after this assignment
Comment 10 Sanjay NM 2014-10-10 05:09:39 UTC
Created attachment 288184 [details] [review]
videomixer: Removed unwanted assignment to start_time

start_time is not being used after assignments in function gst_videomixer2_sink_clip. Removed these assignments
Comment 11 Sanjay NM 2015-02-13 11:37:54 UTC
In the latest code of gstvideoaggregator, this is already included may be by some other patch. So this patch is now obsolete