[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/Makefile stream/tv.c stream/tv.h stream/tvi_vbi.c stream/tvi_vbi.h

[Michael Niedermayer]

Hi

On Sun, Jun 10, 2007 at 10:17:50PM +0200, Diego Biurrun wrote:
> 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
> > >
> > >Added:
> > >   trunk/stream/tvi_vbi.c   (contents, props changed)
> > >   trunk/stream/tvi_vbi.h   (contents, props changed)
> > >Modified:
> > >   trunk/Changelog
> > >   trunk/DOCS/tech/MAINTAINERS
> > >   trunk/DOCS/tech/slave.txt
> > >   trunk/cfg-common.h
> > >   trunk/command.c
> > >   trunk/configure
> > >   trunk/input/input.c
> > >   trunk/input/input.h
> > >   trunk/libvo/sub.c
> > >   trunk/libvo/sub.h
> > >   trunk/mpcommon.c
> > >   trunk/mplayer.c
> > >   trunk/spudec.c
> > >   trunk/spudec.h
> > >   trunk/stream/Makefile
> > >   trunk/stream/tv.c
> > >   trunk/stream/tv.h
> > 
> > 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,
> - not split the patch into self-contained small pieces,
> - ...?
> 
> Additionally the patch litters MPlayer with a huge number of #ifdefs,
> indentation mixes tabs and spaces inconsistently, documentation is full
> 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.

ive not really looked at the patch/commit but 77k for teletext decoding
with some external lib .... hmm
ive written a teletext decoder in 1999 and that was <22k including
demodulation, error correcting, color, caching of pages and subpages

does this thing here even properly support subpages? ive just seen commands
to select the page number not the subpage number

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-cvslog/attachments/20070611/d704c2aa/attachment.pgp>