[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