gstreamer: baseparse: add getcaps function

[Thiago Sousa Santos]

On Wed, 2011-11-16 at 14:06 -0800, Sebastian Dröge wrote:
> On Mi, 2011-11-16 at 17:21 -0300, Thiago Sousa Santos wrote:
> > On Wed, 2011-11-16 at 10:51 -0800, Sebastian Dröge wrote:
> > > On Di, 2011-10-18 at 04:29 -0700, Thiago Sousa Santos wrote:
> > > > Module: gstreamer
> > > > Branch: master
> > > > Commit: e3f2d7db71dfb14441648033eb6cf3324ccc98b3
> > > > URL:    http://cgit.freedesktop.org/gstreamer/gstreamer/commit/?id=e3f2d7db71dfb14441648033eb6cf3324ccc98b3
> > > > 
> > > > Author: Thiago Santos <thiago.sousa.santos at collabora.com>
> > > > Date:   Mon Oct 17 14:42:08 2011 -0300
> > > > 
> > > > baseparse: add getcaps function
> > > > 
> > > > [...]
> > > >
> > > > +  if (klass->get_sink_caps)
> > > > +    caps = klass->get_sink_caps (parse);
> > > > +  else
> > > > +    caps = gst_pad_proxy_getcaps (pad);
> > > 
> > > Hi,
> > 
> > Hello,
> > 
> > > 
> > > this is completely breaking backwards compatibility with the old
> > > baseparse subclasses. Let's take two examples:
> > > 
> > > 1) h264parse allows to convert between different stream-formats. If
> > > downstream now requires a specific stream-format,
> > > gst_pad_proxy_getcaps() will propagate this singla stream-format to
> > > upstream and if upstream wants to provide something different
> > > negotiation will fail. But it shouldn't, h264parse can convert between
> > > the stream-formats. f3f9e4b9786d00299e0c6909180c4cd326489bdf fixes this
> > > in h264parse but it's nonetheless API/ABI breakage.
> > 
> > There was a patch for that in the h264parse in
> > https://bugzilla.gnome.org/show_bug.cgi?id=661874 waiting for review.
> > Was waiting it to be accepted before using the same approach on other
> > parsers that can do conversions.
> 
> Oh, sorry that I forgot about this patch. Is yours better/different than
> mine?
> 

Your implementation is simpler, I'd go with that.

> > > 
> > > 2) Consider a parser and downstream requests parsed=true on the caps.
> > > gst_pad_proxy_getcaps() will propagate the parsed=true field to the
> > > sinkpad caps and if upstream provides unparsed input negotiation will
> > > fail again.
> > 
> > I guess removing parsed/framed entries on caps can be added as a default
> > handling on baseparse.
> > 
> > 
> > The other option is to revert that patch and add a getcaps to all
> > parsers as the default getcaps gets the templatecaps. Template caps
> > won't have any other downstream restrictions on width/height/framerate
> > propagated.
> 
> I think to be really backwards compatible we have to always return the
> template caps if the subclass didn't implement a getcaps function
> (instead of using gst_pad_proxy_getcaps()). In 0.11 we could change that
> to do something more clever and requiring subclasses to implement the
> getcaps function though.
> 
> Nonetheless it makes sense to implement the getcaps function in 0.10 for
> all existing parsers to propagate the downstream caps.

I agree, let's revert that patch and add getcaps manually to our
parsers. Maybe we could add a generic getcaps convenience function on
baseparse that parsers could point to for a default implementation to
avoid replicating code. Might be enough for the ones that won't do
conversion.

--
Thiago

> _______________________________________________
> gstreamer-devel mailing list
> gstreamer-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel