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 703499 - videomixer: take the newly introduced segment->offset into account.
videomixer: take the newly introduced segment->offset into account.
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-07-02 23:30 UTC by Mathieu Duponchelle
Modified: 2013-09-20 14:39 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
videomixer: take the newly introduced segment->offset into account. (1.09 KB, patch)
2013-07-02 23:30 UTC, Mathieu Duponchelle 		needs-work Details | Review
videomixer: Take the newly introduced segment offset into account. (1.47 KB, patch)
2013-07-03 13:41 UTC, Mathieu Duponchelle 		none Details | Review

Description Mathieu Duponchelle 2013-07-02 23:30:13 UTC 
I observed cases where seeking with SEEK_TYPE_NONE in gnonlin would make
the collectpads have a non negative segment offset.

This caused the conversion to running time to return -1, and thus
to make videomixer assert. In my opinion, the check had to be updated because
that offset is a new introduction.

Also, I think the logic is better, we check start against segment->start and
stop against segment->stop, instead of the weird logic that was before and
that actually let one case pass, the one where start_time would be
inferior to segment->start but not end_time.
Comment 1 Mathieu Duponchelle 2013-07-02 23:30:16 UTC 
Created attachment 248264 [details] [review]
videomixer: take the newly introduced segment->offset into account.

Also, make the check for buffer inside segment better.
Comment 2 Mathieu Duponchelle 2013-07-02 23:31:31 UTC 
(In reply to comment #0)
> I observed cases where seeking with SEEK_TYPE_NONE in gnonlin would make
> the collectpads have a non negative segment offset.

Non zero segment offset excuse me.
Comment 3 Sebastian Dröge (slomo) 2013-07-03 06:19:50 UTC 
Review of attachment 248264 [details] [review]:

::: gst/videomixer/videomixer2.c
@@ +710,3 @@
       /* Check if it's inside the segment */
+      if (start_time < segment->start + segment->offset
+          || end_time >= segment->stop + segment->offset) {

Independent of the addition of offset, this is not correct. A buffer can still be inside the segment if the start time is before the actual start (i.e. if the end time is after the start), and also if the end time is after the end (i.e. if the start time is before the end).
Comment 4 Mathieu Duponchelle 2013-07-03 13:41:07 UTC 
Created attachment 248311 [details] [review]
videomixer: Take the newly introduced segment offset into account.

Otherwise, after seeks with SEEK_FLAG_NONE, the conversion to
running time could return -1.
Comment 5 Sebastian Dröge (slomo) 2013-07-17 10:05:31 UTC 
So what does happen here with a SEEK_FLAG_NONE seek? offset becomes the current running time, start becomes 0? What happens to stop?
Comment 6 Sebastian Dröge (slomo) 2013-07-24 06:54:45 UTC 
What should we do about this here? I'm sure there still is a problem somewhere but can't do much without more details
Comment 7 Mathieu Duponchelle 2013-09-20 14:23:08 UTC 
We should close that one I believe, the problem we experienced has been fixed.