[Bug 735626] multipartdemux: Caps are null in pad-added callback.

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=735626
  GStreamer | gst-plugins-good | 1.4.0

--- Comment #2 from Jason <jlitzinger at control4.com> 2014-08-28 18:32:21 UTC ---
<jlitzinger> In Gstreamer 1.4, when a multipartdemux creates a pad, it sets the
pad's caps after creating the pad, whereas in 1.2, the caps were set before the
pad was created.  The effect is that, in the pad-added callback, the caps are
NULL where they weren't before.  Would this be considered a bug, or was the
change intentional?
<__tim> it sounds like a bug to me, but did you check the commit log? :)
<jlitzinger> yeah, I see where it happened, but no mention of this effect
<jlitzinger> I'll get the sha
<__tim> I don't think it was on purpose
<jlitzinger> 9966fdfa756ff2ef5c5dcf541cb9b9dfe3b3d878
<jlitzinger> ok
<jlitzinger> I'll log a bug, and add a proposed patch to it
<jlitzinger> thanks!
<__tim> thanks
<__tim> yup
<__tim> stormer, ^^
<jlitzinger> Actually, it does look intentional...
<jlitzinger> The change was to ensure a stream-start event is sent prior to
caps, to avoid the ""Got data flow before   stream-start event" warning...
<jlitzinger> That begs the question, should an application ever expect a pad to
have caps when the pad is added?
<__tim> well, it's not a hard requirement
<__tim> if you know the caps, you should make them available before adding the
pad
<__tim> if those events get sent on the not-yet-added pad, that's fine, they
will be stored in the pad as 'sticky events'
<__tim> and sent as soon as the pad gets linked
<jlitzinger> Looking at the multipartdemux code, it knows the caps, however,
1.4 change intentionally defers their transmission to avoid the "Sticky event
misordering, got 'caps' before 'stream-start'" warning.
<stormer> __tim: jlitzinger:  yes, sound like a bug/regression, some more code
might have been needed
<stormer> jlitzinger: where does it fail ?
<__tim> jlitzinger, as I said, I don't think it was intentiontal to affect the
way caps are handled, the point of the patch was to send a stream-start event
before setting the caps
<__tim> jlitzinger, please just move the add_pad back to where it was (or I can
do it if you like) :)
<jlitzinger> Heh, I did that, and that's what led to my second question.
<jlitzinger> If I add it back, the original warning message comes back
<stormer> jlitzinger: maybe I shouldn't have move where the pad is added ?
<jlitzinger> stormer:  In the pad-added callback, the caps are NULL in 1.4 for
multipartdemux
<__tim> jlitzinger, oh really? that's weird, could you open a bug then for
starters, attachj your patch and then we'll investigate some more
<jlitzinger> yep
<__tim> thanks
<jlitzinger> working on it now, just wanted to test first
<stormer> __tim: I see the bug in the code, that would mean in general, we need
to stick the stream-start, then caps, and then add the pad
<stormer> jlitzinger: are you setup to give it a try, remove
gst_element_add_pad (GST_ELEMENT_CAST (demux), pad);, and put it back to it's
original location
<stormer> ah wait
<stormer> __tim: hmm, I'm stuck I guess, gst_pad_create_stream_id() require me
to be parented, and setting caps before setting stream-start throw a warning
<stormer> s/me/the pad
<jlitzinger> I did this
<jlitzinger> http://pastebin.com/hY6U3AqH
<jlitzinger> It works as previous, but I don't think it is what is desired
<jlitzinger> as you said, it seems like the events need to be added in the
order: stream-start, caps
<jlitzinger> All I did was basically preserve the old order, and the old bug :)
<stormer> yes, that would undo the change ...
<jlitzinger> yep
<stormer> slomo might know how to handle this
<stormer> hmm
<jlitzinger> finishing the bug report..
<jlitzinger> I'll omit the patch
<stormer> if gst_pad_create_stream_id() didn't need a parented pad, it would
all be easyer ...
<stormer> would simply have to stick the two events, and then add the pad ...
<jlitzinger> https://bugzilla.gnome.org/show_bug.cgi?id=735626
<stormer> thanks by the way
<jlitzinger> np
<jlitzinger> Should I copy/paste the relevant parts of this chat into the bug?
<__tim> sure

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.