[gst-devel] recent wave of element additions

[Thomas Vander Stichele]

Hi,


> I'm a bit afraid by the recent wave of element additions. The main issue here 
> is maintainability, QA and completeness of those elements.
> We have a lot of elements in both plugins and the core that are poorly 
> maintained (if at all), because they are rarely used. And then if somebody 
> starts using them, he figures out they are broken. Good examples here are 
> aggregator, which is a core element that is broken since 0.7.x and shout2send, 
> which just segfaulted until this release. The next problem is that lots of 
> elements are incomplete, especially wrt event handling. This is especially bad 
> because the default action is to pass on the event unmodified, which leads to 
> event handling looking as if it works while in reality it doesn't.
> 
> Most of the time people writing a plugin only it to do one particular job, 
> like playback a gst-launch line. Noone will ever send an event there so it's 
> not an issue. So I can understand why nobody would bother implementing it in 
> that case. I have nothing against such plugins per se but I'd prefer people 
> not putting them in our release-quality code and especially not in the core 
> unless there's a big amount of applications wanting it so it justifies 
> inclusion.

multifdsink got moved to the plugins.  As for other elements; I'd agree,
but it'd involve us going over all elements.  It makes no sense to me to
favour old already present elements that have lost their maintainer over
newly added elements who by definition obviously are maintained.

> Just for the record, here's what I've found from glancing over recent 
> additions:
> - audiorate doesn't handle events, so for example audiorate->in_offset may 
> appear grossly wrong after a discont.

afaict it will do the right thing; the incoming buffer will have
differing timestamps causing it to fill up the gap.

> - audiorate uses g_warning instead of debugging messages

There was one left, fixed in CVS.

> - audiorate doesn't use GstAudioFilter

I don't think it was really decided what GstAudioFilter's target was,
but as I understood it it was meant for in-place transformation of audio
data without changing properties of it.  Maybe when audiorate actually
does interpolation to fill up gaps or adjust the rate it would make
sense to implement it that way.

> - videodrop and videorate seem like duplication

They are, but they do some things differently and it seems to me at
least that videorate handles more cases.  Anyway, it's up to Ronald to
decide; he already hinted that videodrop can go if videorate handles all
those cases, so when Ronald has tested videorate for the cases he used
videodrop we can remove it.

IIRC Ronald wrote videodrop to replace the autofps stuff in v4l which
should go out.

> - videorate doesn't use GstVideoFilter

Esp. for this one it's not doing inplace transformation, so I don't
think subclassing GVF would be appropriate.

> - videorate doesn't handle events, so seeks by default units (number of 
> frames) go wrong

if it doesn't pass them on correctly, it's a bug that needs fixing.

> - videorate uses debugging output, but has no category.

was already fixed in CVS.

> - videobox uses gst_buffer_new_and_alloc instead of gst_pad_alloc_buffer

I think Wim was planning on fixing that - Wim ?

> - videobox doesn't handler events, like navigation

that would be a pain to get right I'd imagine, but yes, needs fixing.

> - videobox isn't subclassed from GstVideoFilter

srccaps != sinkcaps

> - get/set_property on multifdsink is unimplemented
> - the algorithm in multifdsink of dropping all blocking fds when one of them 
> is writable seems very suboptimal for what people expect

both of these are fixed in CVS.

> - a multifdsink should be implemented as tee ! fdsink  tee0. ! fdsink  ... 
> anyway, instead of being a core element

We've been through this.  It is by definition more overhead to create
1000 fdsinks for 1000 clients when one element can do it.  All that
could be discussed is whether the overhead is small enough to not make
it an issue.  We believe it is trivially obvious - if you don't, please
provide us a test case that handles 1000 clients with your approach
working as nicely as Wim's.


> While some of them are only minor issues, others can introduce bugginess or 
> aunexpected behaviour. And we have to be particularly careful here, because 
> GStreamer is known for two things to the general public: powerful and non-
> working. I only like one of those.
> 
> So how do we handle this?

a 0.9 series code review where we decide what to do with each and every
plugin ?

Thomas


Dave/Dina : future TV today ! - http://www.davedina.org/
<-*- thomas (dot) apestaart (dot) org -*->
I won't leave you
all you have is that spell
cast it will you
<-*- thomas (at) apestaart (dot) org -*->
URGent, best radio on the net - 24/7 ! - http://urgent.fm/