[MPlayer-dev-eng] Re: stupid option handling.. again???

[Alban Bedel]

Hi Andriy N. Gritsenko,

on Fri, 11 Apr 2003 19:08:39 +0300 you wrote:

>     Hi, Alban Bedel!
> 
> Sometime (on Friday, April 11 at 15:16) I've received something...
> >>    I did that to let each filter register itself so it can be visible
> >as>soon as it developed. No changes need at filters at all, exempt
> >config>structure (but you did it too, isn't it?). Changes for new
> >config scheme>are even less than yours.
> >Yes, but drawbacks are much bigger with your solution.
> 
>     It seems you won't even look for my patch if I don't split it to
> bunch of small ones. I see. OK, I'll do it after weekend. I'll send its
> one by one then. ;)
> 
> >>    Ok, we can let filters be undefined and parsed on run. But then we
> >>cannot reach and configure them from Gui. One way to do it is to
> >describe>it in Gui explicitly. Another point of registering filters is
> >to allow>set its parameters in config file. Now you can (I didn't
> >chacked if even>that is possible, may be not) only describe full -vf
> >(or -vop) line>there. It's bad for me, I don't like that way. You can
> >solve both these>problems (and don't mentioned yet a problem with
> >modularized filters that>may come in the near future) by registering
> >from vf.c. What's wrong with>it?
> >Dependency. stfa.
> 
>     Any file in these sources already has a bunch of dependencies, and
>     it
> has dependencies on m_* too - just see all sources. That registering is
> not more than function to prepare all codecs (or filters) to using its
> in MPlayer/MEncoder.
> 
> >>Ok. I think just as developer. My reasons:
> >>1) when I add new filter/etc. then why I want to know every point
> >where>  that filter is mentioned? I want just add the file and include
> >it in>  main list. It's a little problem and that is why some filters
> >are>  missed in documentation yet. Sorry, but it's inconvenient.
> >Since when do you have to do more than putting the filter in the main 
> >list to add a filter ?????
> 
>     -ovcopts,-divxopts,... Enough? If I want add new codec I have too
> much of headache instead of just adding it. With -vf you have solved
> that problem by creating new type of option and adding about 10kB of C
> code but it still cannot be visible outside of the vf.c. How about other
> stuff here, eh? ;)
> 
> >>2) let's think if I want to modularize that stuff. In current project
> >I>  cannot do it at all - I have all my code linked. If I do anything
> >like>  a loader then I cannot get any configuration for any modules
> >since it>  needs static links to main config.
> 
> >There is really no advantage. Again f we use registering then the 
> >register function (and all his deps) have to be linked in. If we just 
> >export a var the host app can chose what to do.
> 
>     Why linked??? Register function is in main filters file (vf.c for
> example) only! You can link your filter/codec later with dlopen() and
> get all config structure for using by Gui on the fly! Your current
> solution never allows it!
I know that registering is needed if you dlopen some stuff. Again i know
the advantages of registering (i wrote this registering stuff after all)
but we experienced some problem with that. Registering is ok in the host
app (here mplayer.c mencoder.c) others must export options list and not
call the register function themself.

>     As Balatoni Denes said in answer to you, what I propose does really
> allow write a filter by third party and registering is worth it. :)
For dlopend modules that's the only way and that's partialy why the
registering function are still there.

> >If it want to use this stuff it can by using the exported vars (as
> >mplayer is doing). If it doesn't care then it just have to ignore these
> >vars.
> 
>     I hate externs at all since it's source of most of bugs/unresolved
> conditions/extra dependencies. Good program will have only minimal
> number of extern declarations and only in one header file. :)
>     For example, screen_size_xy is defined in libmpcodecs/vd.c, declared
> in cfg-common.h but cfg-common.h is never included in libmpcodecs/vd.c.
> It's a bad way of programming, I never did it in my own programs.
>     Actually, you can have tons of includes in each *.c file but I don't
> like it as I said, and these are unneeded dependencies.
I hate doesn't like global vars either. With the first system and
registering i also wanted to kill a lot of unneeded globals but it's
turned out to be really not worth it.

>> >>    Thank you, I really missed something in it, now (for week now) I'm
> >>working on it. I'll discuss it with you when it ready, ok? :)
> >>    Anyway if you use multithreaded program something may be wrong if
> >>someone will change config of running filter, isn't it? I think if we
> >get>MPlayer/MEncoder multithreaded then we have to rethink it all. :)
> >MPlayer/MEncoder will NEVER be threaded as it's one of the core
> >feature.
> 
>     So I did it that way. :)
> 
> >But some parts (like libmpdemux) are meant to be usable without (most
> >of) the mplayer code base. So these could happend to be used in a
> >multithreaded prog (hence it would be nice to have them fully
> >reentrant).
> 
>     Reenterability is reached in my local version yet, as I said above,
> I'm working on it now.
> 
> >Also note that m_config is only meant to configure stuff at startup not
> >while mplayer/mencoder is running.
> 
>     I very am not agree with you in that point. My first goal was to use
> m_config_t structure in runtime. And I've reached that goal. :)
By this i meant that when mplayer is playing a movie the config system
shouldn't modify the options. You can set the options and so on only when
mplayer isn't using these variables otherwise you risk a lot of bugs ;)

> >We may latter add support for runtime options settings in filter but
> >that don't have anything to do with the m_config stuff.
> 
>     By adding another alternate parser? Why we have to have two, three
> and so on parsers in the code? It sounds as nonsence for me, sorry. :)
No, but the stuff that may be set dynamicly must be marked as is. It's
really stupid if you just assume that you can modify any option as you
want at runtime.

> >>    As I said before, new extra "desc" field is for "nice names" for
> >Gui,>also it give you a nice commandline help. About 'widget hints'
> >above: Gui>already know which widget you have to use - see my previous
> >letter how>field type has to be interpreted in Gui. :)  The main goal
> >of my patch>was to give Gui all need hints
> >I have nothing aginst such thing but then add only one field for help
> >msg, gui hints, etc. As you probably noticed adding fields to the 
> >m_option struct isn't a so good idea. So if possible this time should
> >be the last time ;)
> 
>     OK. That patch will be first of bunch of ones I'll send here. ;)
> 
> >>    Heh. I don't like to push all authors - since my way doesn't touch
> >a>syntax (what I don't like at most in your current way, sorry, and I
> >think>most of users and developers also hate changes of syntax), it's
> >not need>to wait, all filters will use the same syntax now as before,
> >even if you>will move parsing from sscanf() to config parser. :)
> >Again i didn't changed the syntax. Pls point me to a vop string wich
> >was working and have now a different behaviour (except Gabu
> 
>     May be I missed something but in earlier message you wrote that -vf
> syntax was changed alike filter1=par1=val1:par2=val2,filter2...
and that the old filter1=val1:val2 also work and that you can even mix
the 2.

> >>>I understand that it probaly take you a lot of time for this patch
> >but i>>can't accept it. Also the parts in libvo, libao, etc where you
> >register>>the options won't either be accepted by the maintainers of
> >these parts>>of mplayer (bcs of the unneeded dependency it introduce).
> 
> >>    Let's ask the maintainers of these parts? :)  BTW, what is
> >dependency>that you said about? You meant m_config* stuff? But you
> >already changed>many in libmpcodecs, I don't think it's a problem with
> >any other stuff. It depend only on m_option/m_struct wich is imho 'ok'.
> 
>     In my way it depends only on m_config.h that is almost the same. :)
> 
>     BTW, I'm more and more thinking of modularized structure with
>     minimal
> dependencies between parts. But it seems it may be possible only in next
> generation of mplayer...
Perahps but imho there is still a lot that can be done to modularize
mplayer.
	Albeu