[MPlayer-dev-eng] [PATCH] factorize print_version()

[Uoti Urpala]

On Mon, 2009-01-12 at 13:02 +0100, Diego Biurrun wrote:
> On Mon, Jan 12, 2009 at 03:45:26AM +0200, Uoti Urpala wrote:
> > On Mon, 2009-01-12 at 02:01 +0100, Diego Biurrun wrote:
> > > Here is a patch to factorize the version string etc. output in mplayer.c
> > > and mencoder.c.
> > 
> > Do you have any motivation for this beyond just decreasing code
> > duplication?
> 
> I stumbled across it while looking at the copyright year update patches
> and the bugzilla entry 1378 about some information about CPU options not
> being printed.  Yes, the motivation is to decrease the code duplication
> and to avoid bugs like 1378 in the future.
> 
> > I don't really like it - I think MPlayer code quality is
> > the most important thing, and from that viewpoint this way of splitting
> > code between mplayer.c and mpcommon.c is IMO quite arbitrary and does
> > not give a good architecture. If MEncoder is developed further then that
> > would be best done by deleting most of its independent code and moving
> > the encoding functionality to work with MPlayer's frame generation. If
> > that is done it doesn't matter much what extra crap mencoder.c had.
> 
> I don't think MEncoder should be left to rot even more.  The way of code
> splitting can be changed at any point easily.  The code duplication
> causes issues now.

The code can be moved back later reasonably easily, but is it really
beneficial even in the short term? MEncoder is quite rotten already. As
long as MEncoder is not really maintained MPlayer quality is what
matters, and IMO it's worsened by changes like this.

MEncoder has lots of duplicated functionality, much of it an inferior
version of the MPlayer code. That's exactly why encoding should be
implemented on top of MPlayer's frame generation if MEncoder is to be
developed further. Splitting random pieces of MPlayer between "common"
and "non-common" files and using those in some individual parts of
MEncoder isn't really getting any closer to that goal.