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 748054 - avfvideosrc: starting capture session too early, sending buffers with PTS=NONE
avfvideosrc: starting capture session too early, sending buffers with PTS=NONE
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal major
: 1.5.1
Assigned To: Ilya Konstantinov
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-04-17 13:45 UTC by Ilya Konstantinov
Modified: 2015-05-11 11:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
avfvideosrc: drop frames we get before we have a clock (952 bytes, patch)
2015-04-17 13:49 UTC, Ilya Konstantinov
committed Details | Review
avfvideosrc: fix unconditional buffer queue unlock (1.31 KB, patch)
2015-05-07 19:23 UTC, Ilya Konstantinov
committed Details | Review

Description Ilya Konstantinov 2015-04-17 13:45:25 UTC
avfvideosrc starts the capture session earlier than PLAYING, and sample buffers that arrive before element has clock are stamped with pts=GST_CLOCK_TIME_NONE.

Whether or not we're interested in starting session early (for whatever reason...), we should not send out those buffers.

After http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?
id=5bc17dac295da9ec33c05b42ddac41b61bfaaaba, we now don't try to use clock anymore, but we still end up sending buffers with pts=GST_CLOCK_TIME_NONE which seems bogus.
Comment 1 Sebastian Dröge (slomo) 2015-04-17 13:47:41 UTC
Especially the first buffer timestamps should never be NONE. That's confusing sinks.
Comment 2 Ilya Konstantinov 2015-04-17 13:49:25 UTC
Created attachment 301839 [details] [review]
avfvideosrc: drop frames we get before we have a clock
Comment 3 Sebastian Dröge (slomo) 2015-04-26 18:28:09 UTC
Review of attachment 301839 [details] [review]:

::: sys/applemedia/avfvideosrc.m
@@ +852,3 @@
+  if (timestamp == GST_CLOCK_TIME_NONE) {
+    [bufQueueLock unlock];
+    return;

Don't we have to release sampleBuffer here?
Comment 4 Ilya Konstantinov 2015-04-26 18:34:21 UTC
(In reply to Sebastian Dröge (slomo) from comment #3)
> Don't we have to release sampleBuffer here?

No, we're in the delegate and we haven't retained it.

https://developer.apple.com/library/ios/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html

"A corollary of the basic rules is that when you pass an object to another object (as a function parameter), you should expect that the receiver will take ownership of the passed object if it needs to maintain it."
Comment 5 Sebastian Dröge (slomo) 2015-04-26 18:37:45 UTC
commit 528871f571b83b399ecc352a4e8358ad605aeb8b
Author: Ilya Konstantinov <ilya.konstantinov@gmail.com>
Date:   Wed Apr 15 01:24:45 2015 +0300

    avfvideosrc: drop frames we get before we have a clock
    
    https://bugzilla.gnome.org/show_bug.cgi?id=748054
Comment 6 Ilya Konstantinov 2015-05-07 19:23:15 UTC
Created attachment 303048 [details] [review]
avfvideosrc: fix unconditional buffer queue unlock

My previous patch introduced a bug -- we should never unlock unconditionally unless stopRequest is set.

Additionally, the same bug was present in the unlockStop method.
Comment 7 Ilya Konstantinov 2015-05-07 19:23:54 UTC
(This was discovered by setting the element into PAUSED then PLAYED.)
Comment 8 Matthew Waters (ystreet00) 2015-05-11 11:20:19 UTC
commit c948484c7fe764467ee749aa04d02617d8fee591
Author: Ilya Konstantinov <ilya.konstantinov@gmail.com>
Date:   Thu May 7 21:18:27 2015 +0200

    avfvideosrc: fix unconditional buffer queue unlock
    
    Unless stopRequest is set, we should unlock conditionally -- otherwise,
    the 'create:' method can wake up to an empty buffer queue
    and pull a nil buffer.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=748054