[MPlayer-dev-eng] [PATCH] dvdnav
Benjamin Zores
ben at geexbox.org
Thu Jul 26 14:27:42 CEST 2007
Ötvös Attila a écrit :
> Hi All!
Please Ötvös,
I know you're working hard on it and hope to have DVDnav feature
implemented but if you would just stick to the rules, that would help
everyone review the patches and help them being commited.
First, please SPLIT YOUR DAMN PATCHES by features !!!
> This patch implements the still and wair frames and handler to audio and spu.
That's the problem !
We don't want one massive monster patch.
Reading this sentence, I expect 4 patches :
- still frame handler
- wair frame handler (WTF is that by the way ?)
- audio handler
- spu handler
It's pretty hard to see which line of code affects which feature in your
patch.
Besides, your patch is full of #if 0 / #endif code.
If you know it's useless, don't put it in, it's that less to be reviewed.
> Remark: to the correct buttons required navreadpriv.patch in libdvdnav2
> (r950).
> (http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/2007-July/000212.html)
Which is a patch that was rejected by Nico on this list so don't base
your code on it.
Ben
More information about the MPlayer-dev-eng
mailing list