[gst-devel] multifilesink and proposal to merge it with filesink

[Zaheer Abbas Merali]

On Sun, 2004-07-25 at 13:26 +0200, Ronald S. Bultje wrote:
> Hi Zaheer,
> 

Hi Ronald,

> let me try to explain it a bit clearer. There's currently a bunch of
> plugins added to gst-plugins that *might* at *some point in time* be
> useful to other applications. However, currently, they're not. They're
> ususally being hacked up to only work with specific pipelines (simply
> because they don't do proper event handling, don't have proper state
> reset functions, etc.). The consequence of this is that these elements
> are maybe not *in theory*, but they are *in practice*
> application-specific. Other applications will not use them anytime soon.

Firstly, multifilesink has the same state reset functions and event
handling (except for new media discontinuities) that filesink does.  In
essence it is a small patch to filesink and that is why I suggested it
ro be merged into filesink.  So if filesink does not have proper state
reset/event handling then yes, multifilesink has the problems you state.

Also I have committed a testsuite source example that tests the 2 app
interfaces.

I have 2 testsuite examples in my local tree testing the way it
interacts with multifilesrc (for which the new media change has been
committed) and the way it interacts with wavenc (for which I have not
committed the new media stuff pending further testing).

multifilesink also can be tested in gst-launch.

> Some developers have suggested that these plugins should not be
> committed into gst-plugins until they have wider use. There are various
> reasons to believe that this is a good idea:
> * the plugin is now not expected to work in any application anymore, but
> only in that one single application. This prevents bug reports due to
> poor event handling, state reset handling and so on.
> * the plugin interface (the way in which plugin and application
> interface; properties, interfaces, signals, etc.) is not public anymore,

I admit it was because I had a need for it in acast, that I wrote
multifilesink.  But I consulted Wim and Benjamin with my ideas on how
the interface should be and incorporated suggestions from them.  If you
have constructive ideas on how it should interface to apps etc., please
let me know.  I could easily have spent a lot less time on it if I only
considered the use case of the plugin that acast fits.

> and is thus allowed to change at any point in time. The plugin interface
> of a plugin in gst-plugins may never regress during a stable series.

I do not plan to regress the interface in the 0.8.x series after the
next release of gstreamer or gst-plugins whichever it is proposed to go
into.

> This requires a *stable* and *well-thought-out* plugin interface. We
> doubt that all the recently added plugins are that far. This is not an
> offense to their developers, it's simply deduction: you ususally don't
> get it right the first time, and that's completely normal.

Please look at the code of the recently added plugins, before blanket
accusation.
> * less plugin maintainance and less build time in gst-plugins.
> 

That is a separate issue, and has to be dealt with in a different thread
as to whether to split gst-plugins into smaller modules, and has nothing
to do with multifilesink or the proposal to merge it into filesink.

> Now, I hope that's clear. What I'm saying is: keep it in your
> application, until other applications clearly see the need for this as
> well. Then, we can test and possibly change the plugin interface to suit
> everyone's need, and then possibly add it to gst-plugins. Gst-rec and
> gst-autoplug also have private elements for this same purpose.
> 

I would have kept it private if:

a) I would not have been asked by people how to do x,y,z where x,y,z was
only made possible by such a plugin.
b) It was application specific, which it is not.  It has a general use.
c) I was not proposing it for merging into filesink.

On Fri, 2004-07-23 at 00:10, Zaheer Abbas Merali wrote: 
> > I committed it on the basis of proposing the small changes of adding new
> > media support to filesink.  Also, there have been people on irc asking
> > if such a plugin exists so it would not be unique to one particular
> > application.
> I've added and consequently removed that *ages ago*. See CVS. Reasons:
> * the code became very cluttered and non-obvious. Most of the options
> were not logical in any sane way. The plugin became unnecesarily
> complicated and it basically surpassed the goal of the original, clean
> filesink.

Please feel free to see if the few lines of changes that it has to
filesink makes filesink cluttered and non-obvious.

> * I never figured out a sane way of changing filenames in filesink
> without either doing some evil hackery (in which case it should be an
> application-specific plugin; gst-core stuff is supposed to be
> ultra-clean!), especially in an automated way. In the end, you end up
> filtering and inserting EOS events in the pipeline anyway.
> 

Changing filename can be done in 2 ways:

i) Statically using an index replacing a %d (ie blah%d.wav becomes
blah0.wav on first file, blah1.wav on 2nd file etc.)

ii) Dynamically by the app setting the location property inside the
signal handler attached to the "newfile" signal.

I am not adding EOS events, I am checking for DISCONT events with the
new media flag being set.

> I know it is not application-specific, but it *does not belong in
> gst-core*. Also, it *does not belong in gst-plugins* until we've found
> clean, maintainable ways of solving the above problems. Different people
> have different needs for opening new files and filtering or inserting
> EOS at different points in time. It's *not* as easy and straightforward
> as you described. If it was, I'd have coded it ages ago. Gst-rec needs
> this too.
> 

Again, the GStreamer API specifically shows how new media should be
dealt with and that is by having a DISCONT event with the newmedia flag
set.  Any other way is a hack.

> > I do not understand what you mean by "application-specific interfaces".
> It means that the setup of the plugin to do a specific function is coded
> specifically in a way because one specific application required that,
> rather than being a generic interface that was coded that way because we
> believe that's the right way to solve the issue and have applications
> configure plugins.
> 
> > Remember also that multifilesrc is in gst-plugins and does not have a
> > twin until my commit :)
> 
> multifilesrc is a very general plugin, with a very clean interface to
> the applications. I'm in favour of keeping it. Multifilesink currently
> isn't that yet. Besides, multifilesink and multifilesrc aren't twins
> because they work differently. Multifilesrc creates a new buffer for
> each individual file, which is for example useful if you want to read a
> series of JPEG images and create an MPEG movie from it. Multifilesink
> uses a different approach and creates new files after each individual
> EOS. They're not the same.
> 

Please look at gst-inspect for multifilesrc, it has a really crap (in my
opinion) interface to applications.  The only way to set the list of
files is to give it a pointer to a linked list of filename strings.
This means:

i) it cannot be tested with gst-launch unless there is syntax for
providing properties with GSList* with gchar* as the data type.
ii) there is not a clean way of deciding the next file, think rhythmbox
playlist and a user decides he wants to change order of files.  To
change the linked list in another thread is not a good idea if its not
protected by a mutex that is used in all threads.  Ideally one should be
able to set the next file location in a signal handler attached by the
application.

Again let me reiterate, multifilesink does not create a new file after
every EOS event.  It does it after every DISCONT event with new media
flag set.  My patch for multifilesrc which I committed a few days ago
adds a property to multifilesrc that allows it to send new media
DISCONTs after every file hence actually "fixing" the behaviour if one
so wishes by setting newmedia property to TRUE.  I made teh default
FALSE so it doesnt break existing applications that rely on its current
behaviour.


---
Please can all of you actually look at multifilesink and decide whther
it should be a separate plugin, merged into filesink, or just scrapped
altogether.

Thank you


-- 
Zaheer Abbas Merali <zaheerabbas at merali.org>