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 747517 - appsrc: negotiates twice if caps are changed before pipeline starts
appsrc: negotiates twice if caps are changed before pipeline starts
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-04-08 15:40 UTC by Ilya Konstantinov
Modified: 2015-05-18 12:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
appsrc: remove latest caps if there is no data for the caps. (1.21 KB, patch)
2015-05-18 01:27 UTC, Eunhae Choi
needs-work Details | Review
appsrc: when setting caps twice before being started only use the last caps (6.03 KB, patch)
2015-05-18 10:28 UTC, Tim-Philipp Müller
needs-work Details | Review

Description Ilya Konstantinov 2015-04-08 15:40:31 UTC
On appsrc, before pipeline starts, caps can be set with gst_app_src_set_caps or through the "caps" property.

In both cases, if the caps were already set once, then the new caps will be pushed into a queue that won't be checked before the pipeline starts.

Henceforth, you get the counterintutuive behavior:

  g_object_set (G_OBJECT (appsrc), "caps", caps1, NULL);
  g_object_set (G_OBJECT (appsrc), "caps", caps2, NULL);
  gst_element_set_state(pipeline, GST_STATE_READY);
  /* ^ the old caps1 will be considered during negotiation */
Comment 1 Thiago Sousa Santos 2015-04-09 19:50:39 UTC
It will use caps1 but then at the next iteration will negotiate to caps2, isn't that what happens? It is not exactly counterintuitive as you requested 2 different caps and it will go through caps1 and then caps2. This is useful when you do:

set_caps(caps1)
push_buffer
push_buffer
set_caps(caps2)
push_buffer
...

So what appsrc does is serialize those requests into a queue and process than in the same order, so if you set 2 caps in a row it will push 2 caps event in a row with the requested caps you pass.


Can you describe your use case where that happens? It could be fixed by 'overwriting' caps entries at the end of the queue when setting a new one, but let's understand what you are trying to do.
Comment 2 Ilya Konstantinov 2015-04-11 14:34:18 UTC
My scenario was simply calling g_object_set(appsrc, "caps", caps..., NULL) twice before starting my pipeline (in different parts of my initialization). This wasn't necessary and I'm only making the latter call now, but still it was confusing.

I think overwriting the caps at the end of the queue would solve this.
Comment 3 Tim-Philipp Müller 2015-04-19 16:51:03 UTC
Right, I suspect one could do some kind of "event compression" here.

(Note that what's really counter-intuitive is that you can already push buffers into appsrc before it's started, instead of getting FLOW_FLUSHING :P)
Comment 4 Eunhae Choi 2015-05-18 01:27:52 UTC
Created attachment 303494 [details] [review]
appsrc: remove latest caps if there is no data for the caps.


I've added patch to remove the latest caps if there is no following data when new caps should be set.
Comment 5 Tim-Philipp Müller 2015-05-18 10:28:15 UTC
Created attachment 303514 [details] [review]
appsrc: when setting caps twice before being started only use the last caps

I made a patch for this as well over the weekend, but mine includes a unit test. I think it may also be more correct, because I unref caps with gst_caps_unref() instead of gst_object_unref() and also only do the replacing if we haven't started yet (otherwise we have to queue the caps in order to trigger the actual renegotiation as far as I can see).
Comment 6 Tim-Philipp Müller 2015-05-18 12:18:09 UTC
Actually, ignore my comment about pushing the caps into the queue, your patch does that always too, so there's no problem there.
Comment 7 Tim-Philipp Müller 2015-05-18 12:44:17 UTC
Pushed this now:

commit 8304893d061d4a134ea87801b710dfb99ae15968
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Mon May 18 11:23:16 2015 +0100

    appsrc: optimise caps changing when previously-set caps have not taken effect yet
    
    Only negotiate/change caps once when setting caps twice and
    the first-set caps have not been used yet.
    
    Based on patch by Eunhae Choi.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=747517


I think your approach to check the queue's tail is better and more universal, so I've taken that over, but fixed the unref. And removed the !started check from my patch then.
Comment 8 Tim-Philipp Müller 2015-05-18 12:48:58 UTC
Comment on attachment 303514 [details] [review]
appsrc: when setting caps twice before being started only use the last caps

Committed with changes.