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

[Andriy N. Gritsenko]

    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!
    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. :)

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

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

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

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

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

    With best wishes.
    Andriy.