[gstreamer-bugs] [Bug 631574] Add GstFilters library to gst-plugins-base

Tue Dec 21 08:35:41 PST 2010

https://bugzilla.gnome.org/show_bug.cgi?id=631574
  GStreamer | gst-plugins-base | git

--- Comment #10 from Sebastian Dröge <slomo at circular-chaos.org> 2010-12-21 16:35:36 UTC ---
(In reply to comment #9)
> (In reply to comment #7)
> > Just some high-level comments from looking at the docs
> > 
> > - Why do you have all these volume/audioconvert/etc filters? Isn't this
> > essentially the same as a GstBinFilter with the corresponding description? I'm
> > not sure that these are really needed... of course they don't hurt either :)
> [...]
> 5- having them doesn't hurt :)

Ok, fine :)

> > - GstResolutionFilter should also have something to specify the PAR/DAR or to
> > force keeping the DAR and things like that. And the name could be improved I
> > guess... but I don't know a good name
> That's a good point, The aspect ratio should be configurable, I'm not sure if
> it needs to be anything fancy, maybe just 'stretch/keep_aspect_ratio' is
> enough.

Maybe just expose the add-black-borders property of videoscale and maybe four
ints to give a PAR range

> About the name, I don't know, maybe GstVideoSizeFilter ?

Sounds better :)

> > - GstFilter::handle_message(): please add to the documentation *which* messages
> > will be passed to this. Is this only for gst_filter_manager_handle_message()?
> > If it is, what exactly is the use case for this? An example in the
> > documentation would help I guess :)
> 
> Well, the documentation says "Try to handle a message originally received on
> the GstBus to the filter." and really, it's *ANY* and *ALL* messages that goes
> through the bus.. the filter will return TRUE/FALSE depending on whether or not
> it recognizes the message. Most filters don't need it and will always return
> FALSE, others will look for a specific message (like the 'level' filter that
> takes 'level' messages and transforms them into gobject signals) but you can't
> [...]
>
> Does that answer your question or did I misunderstand it ?

No, makes sense. But maybe multiple filters want to get a single message?

> > - It might make sense to use GstObject as base class for GstFilter and
> > GstFilterManager. This way you will get a) the mutex, b) parent/childs
> > handling, c) floating refs
> No, I didn't want to do that for multiple reasons, first of all, the purpose of
> all this is mainly : "Make it easy".
> Gstreamer is scary to a lot of people who have never used it, so I wanted to
> keep this into the realm of what everybody knows by using GObject instead of
> GstObject. I absolutely do not want floating refs, although I completely agree
> that they are extremely useful and practical, I think that they add a certain
> level of complexity to the refcounting system that could easily scare/confuse
> people. Also there is no parent/child system here, a filter can be added to as
> many filter managers as you want, so it's more of a "contains" rather than "is
> a parent of". Finally, adding a mutex is easy, so it doesn't really warrant a
> move to GstObject.
> For all these reasons, I prefer it to be GObject as a base rather than
> GstObject, I don't see any particularly important advantage in using GstObject
> apart from the obvious "This is part of GStreamer so it has to be a GstObject"
> argument.
> What do you think ?
> 
> p.s: by the way, GObject also has support for floating refs.

Makes sense too, especially because it's not really a parent/child relationship
here :)

> > - You should add a note to the documentation that dynamic pipeline changes are
> > not handled in all cases yet
> What do you mean? what cases aren't handled for dynamic pipeline changes? It's
> been a while so maybe it's just that my memory isn't fresh, but as far as I
> remember, there were no issues with dynamic pipeline changes and all cases were
> handled correctly.
> The only thing I can see from this bug report's comments is about changing the
> source/sink elements, but wouldn't changing the source only affect the sink
> synchronization? the filters in between shouldn't really be affected since they
> only transform data and it's rare for a filter (non-src/non-sink element) to
> care/modify the timestamps ?
> The only filter that has a sink in it is the Preview filter and that one uses
> sync=false so it shouldn't bother it either.

See at the end of the comment

> > - gst_filter_add_standard_element() is badly named. Elements with one pad named
> > "sink" and one pad named "src" are not really "standard elements". More
> > something like filter elements, one-to-one elements, etc.
> hehe, well, all the elements that get added are basically filters with
> one-to-one pads, but since this is to be used internally for a filter, it
> doesn't really make sense to call it gst_filter_add_filter_element. I know
> there isn't really a 'standard' but it's become a 'kind of standard' for people
> to name their pads 'src' and 'sink'.. I'm sure if you see an element with a
> sink pad named "my_sink_pad", you would frown during code review and tell the
> committer to name it correctly.
> If you have a better suggestion for the function name, let me know though, I
> don't mind changing it.

Unfortunately I don't know a better name, sorry :) Someone else?

> (In reply to comment #8)
> > (In reply to comment #5)
> > > How would you suggest telling the elements in the filter manager about the
> > > changed source/sink ? The filters (and filter manager) have a _handle_message
> > > API method, so maybe sending some kind of message through that ?
> > 
> > I think the elements in the filter/filter manager shouldn't even notice that
> > the source/sink was replaced, other than maybe seeing a newsegment event. But
> > now that I looked at the docs of GstFilter it seems that this is only meant for
> > elements between the source and sink and not for the sinks/sources themselves.
> > In that case ignore it for now :)
> > 
> Yes, the filters are 'filters' in the gstreamer sense, one input = one output,
> so you can't really use a GstFilter as a source/sink (but a filter could have
> an internal src/sink like the Preview filter)
> 
> > What remains is that you need to tell newly added elements to a running
> > pipeline about the current running time, otherwise you're going to break QoS
> > and synchronization in some cases.
> 
> Humm, the whole timing/synchronization/segment/latency/QoS stuff in gstreamer
> is my weak point, I'm having a bit of a hard time getting my mind around it.
> I'm a bit clueless as to why this is needed, or how to do it properly, could
> you enlighten me?
> If you have a solution that I can just apply, I can fix it, but figuring out
> what needs to be done is giving me a headache...

Essentially you need to install buffer and event probes on sinkpad, catch
newsegment events and buffer timestamps/durations. And then you'll be able to
calculate the current running time when changing elements during playback. And
to tell a new element about the old running time you have to send a newsegment
event to it before anything else happens (and important: drop it on the other
side of the element).

-- 
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.