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 744096 - compositor, videoaggregator: ignores input timestamps if input buffers have no duration
compositor, videoaggregator: ignores input timestamps if input buffers have n...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal major
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-02-06 13:56 UTC by Tim-Philipp Müller
Modified: 2015-02-10 03:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
videoaggregator: fix buffer selection when duration=-1 (1.75 KB, patch)
2015-02-08 14:07 UTC, Matthew Waters (ystreet00)
reviewed Details | Review

Description Tim-Philipp Müller 2015-02-06 13:56:36 UTC
If input buffers have no duration set on them, as is usually the case with live sources (rtp, v4l2src), compositor seems to just ignore the input buffer running time completely and the mixed output is completely bogus. This makes it very clear (but it happens in real live as well if input framerate != output framerate, obviously causing a/v sync issues if audio is involved): 

$ wget http://people.freedesktop.org/~tpm/samples/bipbop-gear4.ts
$ gst-launch-1.0 uridecodebin uri=file://`pwd`/bipbop-gear4.ts name=d ! queue ! timeoverlay time-mode=running-time font-desc=Sans,36 ! compositor ! timeoverlay time-mode=running-time font-desc=Sans,36 valignment=bottom ! videoconvert ! queue ! video/x-raw,framerate=1/1 ! xvimagesink
Comment 1 Tim-Philipp Müller 2015-02-06 14:08:05 UTC
*real life ;)
Comment 2 Matthew Waters (ystreet00) 2015-02-08 14:07:00 UTC
Created attachment 296379 [details] [review]
videoaggregator: fix buffer selection when duration=-1

This fixes the above pipeline and the framerate=100/1 pipeline by taking into account the input buffer times.
Comment 3 Sebastian Dröge (slomo) 2015-02-08 15:55:34 UTC
Review of attachment 296379 [details] [review]:

I think independent of this, videoaggregator should also not do any calculations with the framerate at all. It's never going to give useful results.


Also we probably need different behaviour for live than for non-live pipelines (if input durations are -1). In live pipelines you want to take the newest buffer that is available and does not start after the end time of the output buffer, even if you didn't receive any newer buffers to be able to calculate the duration of the previous buffer. In non-live pipelines you always want to wait until the next buffer so you know the duration of the previous one and know which of the two to place into the current output buffer.

::: gst-libs/gst/video/gstvideoaggregator.c
@@ +1002,2 @@
       if (end_time == -1) {
+        if (start_time > output_end_time) {

output_end_time is in rnning time, start_time a buffer timestamp. That seems not ideal to compare :)

@@ +1006,3 @@
+          gst_buffer_unref (buf);
+          continue;
+        } else if (start_time < output_start_time) {

Same here
Comment 4 Matthew Waters (ystreet00) 2015-02-09 01:02:33 UTC
(In reply to comment #3)
> Review of attachment 296379 [details] [review]:
> 
> I think independent of this, videoaggregator should also not do any
> calculations with the framerate at all. It's never going to give useful
> results.

How do you calculate output buffer start/duration then?

> Also we probably need different behaviour for live than for non-live pipelines
> (if input durations are -1). In live pipelines you want to take the newest
> buffer that is available and does not start after the end time of the output
> buffer, even if you didn't receive any newer buffers to be able to calculate
> the duration of the previous buffer.

So you want to put the latest buffer into the output buffer.  The only way you can reliably do that is by waiting until you get a newer buffer on the sink pad that is ahead of the end time or wait until the timeout runs out.

> In non-live pipelines you always want to
> wait until the next buffer so you know the duration of the previous one and
> know which of the two to place into the current output buffer.

How do you choose which buffer for the non-live case?  The buffer closest to the start? end? middle? random? of the output buffer's timespan.

> ::: gst-libs/gst/video/gstvideoaggregator.c
> @@ +1002,2 @@
>        if (end_time == -1) {
> +        if (start_time > output_end_time) {
> 
> output_end_time is in rnning time, start_time a buffer timestamp. That seems
> not ideal to compare :)

It's what all the other time comparisons do in that function...
Comment 5 Matthew Waters (ystreet00) 2015-02-10 03:47:56 UTC
commit 783245f1fe58d933728ee78ec88315f53b3e04c1
Author: Matthew Waters <matthew@centricular.com>
Date:   Mon Feb 9 00:59:30 2015 +1100

    videoaggregator: fix buffer selection when duration=-1
    
    If the src framerate and videoaggreator's output framerate were
    different, then we were taking every single buffer that had duration=-1
    as it came in regardless of the buffer's start time.  This caused the src
    to possibly run at a different speed to the output frames.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=744096