[gst-devel] new capsnego bugs + re-requesting docs

[David Schleef]

On Thu, Jan 08, 2004 at 01:10:02PM +0100, Ronald Bultje wrote:
> Hi again,
> 
> On Thu, 2004-01-08 at 11:44, Ronald Bultje wrote:
> > filesrc location=file.mpg ! mpegdemux .video_00 ! mpeg2dec ! mpeg2dec !
> > ffcolorspace ! median ! videodrop ! videoscale ! divxenc ! fakesink
> 
> Both median and videodrop are broken. I fixed median. videodrop is more
> annoying. Dave, what's wrong with the attached patch? I get:
> 
> (process:26322): GStreamer-WARNING **: push on peer of pad
> videodrop0:src but peer is not active

This indicates that divxenc0:sink was not negotiated.  For the most
part, "active" == "negotiated".  I'm not entirely sure of the
differences, but the older meaning of "active" was kept.

The first time I read your mail, I missed "attached patch", so I
also fixed it.  Your code was nearly identical to mine.


+  /* try unmodified first */
+  ret = gst_pad_try_set_caps (otherpad, gst_caps_copy (caps));
+  if (GST_PAD_LINK_SUCCESSFUL (ret)) {
+    /* passthrough */
+    videodrop->to_fps = videodrop->from_fps = fps;

I didn't do passthrough, since presumably there is no real passthrough,
and some processing is always done, e.g., "dirty" timestamps are cleaned
up.

+  } else if (ret != GST_PAD_LINK_DELAYED) {
+    /* there is a filter somewhere, let's try to find that */
+    GstCaps *peercaps = gst_pad_get_caps (GST_PAD_PEER (otherpad));

gst_pad_get_caps() and GST_PAD_PEER() is _always_ wrong when called
from inside an element.  Use gst_pad_get_allowed_caps() instead.

+    gst_structure_get_double (gst_caps_get_structure (peercaps, 0),
+			      "framerate", &modfps);

You can't assume that the allowed_caps are fixed, so get_double()
might fail.  You could check the framerate field in the actually
negotiated caps (gst_pad_get_negotiated_caps()), which is what I
did.  (Actually, I checked for the pad being negotiated, and then
used the appropriate video->{to|from}_fps.)

+    caps = gst_caps_copy (caps); /* copy, we'll change stuff */

(minor nitpick: caps is const GstCaps *, and using a const pointer
for something you own (the return value of gst_caps_copy()) is wierd.)

+static GstCaps *
+gst_videodrop_getcaps (GstPad *pad)

This was identical to what I wrote, aside from style.



dave...