[MPlayer-dev-eng] Re: [PATCH] Profiles support for MEncoder - Upgrade

[Alban Bedel]

On Mon, 23 Jan 2006 15:38:54 +0100
Danny - liste <pi at digitalfantasy.it> wrote:

> Alban Bedel wrote:
> 
> > Sorry for jumping in a bit late. The patch look good, but i must say i don't
> > like very much the idea of having yet another parser. For the devs it's
> > one more to maintain and for the users it's one more sligthly different
> > config file format.
> >   
> 
>     You can use the config file sintax and it work, the one listed in 
> the man page is, IMHO, cleaner.
> 
>     Also the fact that I put only one lavcopt on every line is a choise 
> made to simplify the fact that you can comment one option.
> 
>     You can put a profile with all the lavcopts that affect mpeg4 
> encoding commented (with a #) and test it simply adding or deleting a 
> single #.
> 
> > So i'm suggestinge something very similar but extending the config file
> > parser instead. With the attached patch you can add some "sections" to
> > your config file to define some profile. For example:
> >
> > [mpeg4]
> > ovc=lavc=1
> > lavcopts=vcodec=mpeg4:vbitrate=1200
> >
> > [filter640]
> > vf=spp,scale=640:480
> >
> > Then use with: mencoder -profile mpeg4,filter640 file.avi
> >
> > 	Albeu
> >   
> 
>     In the end what is important is to have the feature in the code. 
> Obviusly I prefer mine ;)

:) Sorry but i will soon commit the attached patchs, unless their is some
other objections. The first implement the profiles and the second move
-list-option in m_config making it work in both mplayer and mencoder
(and list itself :). This belong to another thread but the patch are
incrementals so i kept them together.

>     I try to let the user use the style he/she likes, being extremely 
> permissive on the sintax.

Be careful with beeing too permissive ;) Anyway, as long as their is
correct backward compatbility, i'm all for extending/reworking the
current parser to get a nicer syntax. Making the first = optional
should be trivial.
 
>     Also, being a module not in the parser it's may be more simple to 
> expand it: I would like to have a profile in which, as a parameter, you 
> put the bitrate you like (mencoder -profile mpeg4(1200),ac3(224) file.avi)

I was thinking about that too :) It shouldn't be too hard.

>     For the fact to have another parser I agree with you: it's better to 
> have only one parser (and I'm talking also about the suboptions parser).

Well i don't known much about the suboptions parser, beside that i suspect
it duplicate a lot of code alredy in the "normal" option parsers.

>     Maybe it's time to make a libmpparse with, documented, the various 
> function you can use ?

In a way there alredy is:
* m_option.[ch] m_struct.[ch]
    Low level data handeling: convert from/to string, copy, etc
* m_config.[ch]
    High level configuration: take care of saving/restoring configurations
    for the per file/group options, etc
* parser-*.[ch]
    Parse the config file/command line: check/set the options and build the
    playlist.

But their is no docs (beside some comments in the sources) and the API
is not really trivial :(

	Albeu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: profile-alb-2.diff
Type: application/octet-stream
Size: 11024 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20060123/1d54bb9d/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: list-options.diff
Type: application/octet-stream
Size: 1578 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20060123/1d54bb9d/attachment-0001.obj>