[MPlayer-dev-eng] [PATCH]VO_VDPAU, round 5

Carl Eugen Hoyos cehoyos at ag.or.at
Thu Jan 22 10:00:10 CET 2009


compn <tempn <at> twmi.rr.com> writes:

> [15:35] <Compn> carl got those patches accepted to ffmpeg quickly :)
> [15:36] <Compn> hes waiting for a review of vo_vdpau too 
> [15:36] <Compn> a fourth review that is
> [16:04] <uau> IMO he could have done a better job with that
> [16:04] <uau> what he posted had some rather obvious problems
> [16:05] <uau> feels somewhat like posting dirty patches and expecting
> others to do the actual work to make them usable... (not necessarily
> wrong but i think it could at least be expressed more openly rather
> than asking "for review")

I thought I did that, but I agree after re-reading the mails that "more openly"
would have been possible.

> [16:23] <uau> there were several things that should not have needed
> pointing out
> [16:33] <DonDiego> i also pointed out some issues with carl's patches
> every time he posted them
> [16:33] <DonDiego> and he never made changes..

I found one occurrence of a whitespace ending between three and four and one
additional yesterday: I didn't know how to find them with grep, I tried
grep ' \n'.

I found two occurrences of indentation issues since round four (and fixed
several before).

> did you miss some comments that diego made ? or was anything unclear
> or unsaid?

The file has several hundred lines, I ran through it to find those issues
between every iteration.
Since I except the file to need many non-cosmetic fixes until improvement, I
definitely did not spend maximum time on it (but I did spend time on those
cosmetic issues).

> do some rules need to be put in patches.txt (e.g. k&r rules
> or run patch thru ispell) ?

It seems I missed the ispell part.
Apart from that, if you find those issues, I still find it makes much more sense
to point to them (exactly): They are fixed MUCH faster like that (and I do mean
accumulated time), imo.
Other than that, to point out indentation problems in an mplayer patch is
extremely near to pick on somebody giving the current indentation in many files.

Except for one very helpful review, I had the general feeling that the patch is
simply unwelcome.

> (just trying to shrink the language barrier.)

Thank you, Carl Eugen





More information about the MPlayer-dev-eng mailing list