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 778273 - AVFVideoSource must post a latency message after it renegotiates caps
AVFVideoSource must post a latency message after it renegotiates caps
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.11.1
Other Mac OS
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-07 10:34 UTC by Nick Kallen
Modified: 2018-11-03 14:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Post latency message after setting caps (1.13 KB, patch)
2017-02-07 10:40 UTC, Nick Kallen
needs-work Details | Review

Description Nick Kallen 2017-02-07 10:34:01 UTC
Background:

The AVFVideoSource represents the iphone front- and rear-facing cameras. The cameras can output at various frame-rates and putting a capsfilter downstream will trigger the frame-rate selection during caps negotiation. The frame-rate affects the latency, because for example 30fps leads to a latency of ~33ms.

The bug:

Adding an avfvideosrc to a pipeline in the NULL state works fine because PLAYING the pipeline will calculate latency after all the caps have been negotiated.

However, if you add an avfvideosrc to an already running pipeline, it will renegotiate caps but it will not trigger a latency event after deciding its frame-rate. If the pipeline already had a latency less than 1/frame-rate frames will be dropped.

The fix:

Post latency message after caps have been set.
Comment 1 Nick Kallen 2017-02-07 10:40:29 UTC
Created attachment 345095 [details] [review]
Post latency message after setting caps

This patch was tested on top of bug 777847 , however it should be independent.
Comment 2 Nicolas Dufresne (ndufresne) 2017-02-07 16:17:19 UTC
Looks good to me, though, all source will have the same issue. Maybe we could add gst_base_src_set_latency() that does this for us, and implement the LATENCY query generically ?
Comment 3 Sebastian Dröge (slomo) 2017-02-09 12:05:53 UTC
Yes, having such API in basesrc would definitely be useful. So many sources replicate that already... Let's go with that
Comment 4 Nick Kallen 2017-02-09 16:01:52 UTC
One minor wrinkle in this code is that it's essential that 'gst_caps_replace' be called before latency message is posted. That's easy to deal with in this code (call set_latency after gst_caps_replace rather than as its currently done https://github.com/GStreamer/gst-plugins-bad/blob/master/sys/applemedia/avfvideosrc.m#L708) but if your goal is to make this all abstracted away, I don't think your proposal will work.
Comment 5 Sebastian Dröge (slomo) 2017-02-10 10:48:17 UTC
(In reply to Nick Kallen from comment #4)
> One minor wrinkle in this code is that it's essential that
> 'gst_caps_replace' be called before latency message is posted.

Why is that order important? The latency message is handled asynchronously anyway


The gst_base_src_set_latency() API should basically work the same as in e.g. GstVideoDecoder or GstAudioDecoder.
Comment 6 Nick Kallen 2017-02-10 10:49:32 UTC
Honestly I have no idea why but I tested the code and I'm pretty certain it didn't fix my bug in any other order.
Comment 7 Nicolas Dufresne (ndufresne) 2017-03-28 00:22:25 UTC
It all depends if you use a sync bus handler or a async bus handler in your application to handle the latency message. Though, it no timportant, you can just call gst_base_src_set_latency() after your call to gst_caps_replace().
Comment 8 Nicolas Dufresne (ndufresne) 2017-12-19 01:46:43 UTC
That caps replace thing, this is a potential race between update latency bin function and the caps being replace I guess. We should add some locking to be extra safe. We've been delaying this one a lot, shall we merge it (with maybe some locking around the caps) and revisit later when we have helpers on the base class ?
Comment 9 GStreamer system administrator 2018-11-03 14:04:25 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/517.