[MPlayer-cvslog] r23530 - in trunk: Changelog DOCS/man/en/mplayer.1 DOCS/tech/MAINTAINERS DOCS/tech/slave.txt cfg-common.h command.c configure input/input.c input/input.h libvo/sub.c libvo/sub.h mpcommon.c mplayer.c spudec.c spudec.h stream/Makef

[Vladimir Voroshilov]

Well.
first, I have to say real sorry about this patch.


2007/6/11, Diego Biurrun <diego at biurrun.de>:
> On Sun, Jun 10, 2007 at 11:04:49AM +0200, Nico Sabbi wrote:
> > voroshil wrote:
> >
> > >Log:
> > >Teletext support for tv:// (v4l and v4l2 only)
> > >
> > >modified patch from Otvos Attila oattila at chello dot hu
> > [...]
> >
> > couldn't you split it in self-contained pieces?
>
> This commit breaks a big number of policy rules.  Vladimir, why did you
>
> - give no warning that you would commit the patch,
> - commit to other people's code without approval,
I figured out that submitted patch with deadline (three days ago) was
sent to Andrew Calkin's direct mail. gmail displays all of this mails
(teletext related) in single thread and this was confused me a lot.

> - not split the patch into self-contained small pieces,
> - ...?
1. libvo/*,spudec.c,mpcommon.c (rendering)
2. command.c, input.c (user interface)
3. tvi_vbi.c, tv.c (main stuff)
is that splitting will be enough (parts will be dependant of each
other and will not work/compile separately)?

>
> Additionally the patch litters MPlayer with a huge number of #ifdefs,
> indentation mixes tabs and spaces inconsistently, documentation is full
Hm. I though, i had removed all of tabs (excepts those which will be
inserted into "tabulated" code, keeping tabbing style of existing
file).
all #ifdef's looks reasonable for me.

> of typos and partially buggy, the new files don't come with license
> headers.

> If I had had a chance to review and comment on this beforehand I would
> have requested nearly every build system and documentation line to be
> changed.  I'm also confident there has to be a way to implement this
> more cleanly.
I'll be happy to see/adapt alternative/better/cleaner implementation.

Well.
1. Revert with "commit r23530 breaks policy, aproovements of others
devs was not recevied, code should be cleaned up/fixed"
2. Submit patch again, wait for reviews,...
3. Wait for Michael Niedermayer's implementations suggestions (he said
in another mail that he has written teletext implementation with 22k
code size without external lib some times ago)

ok to proceed?

-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719