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

Carl Eugen Hoyos cehoyos at ag.or.at
Thu Jan 22 15:13:05 CET 2009


Diego Biurrun <diego <at> biurrun.de> writes:

[...]
> > Except for one very helpful review, I had the general feeling that the
> > patch is simply unwelcome.
> 
> I appreciate your efforts around picking up patches and getting them
> ready for inclusion.  However, the process you use can be improved in
> multiple ways:
> 
> - I think I prefer keeping pathces in one thread unless the thread
>   becomes very long.  New threads per round are not optimal.

Some people could live with it...

> - It is not clear what sorts of issues a patch still has when you submit
>   it.

I _thought_ I fixed the issues:
I didn't know how to find terminating whitespace.
I simply missed - imo - two cases of wrong indentation (in >800 lines).

I did not use ispell because I didn't knew it (probably because I read the
patch guidelines not carefully enough).

> - It is not clear what sorts of changes you make between rounds.

I tried to make all, I did mention that I did not fix pan&scan between two
rounds.

> - If you have trouble fixing some issue that gets pointed out, ask.

I did not have trouble, I simply missed some occurrences.
Apart from that, I did not fix one (not so small) case of code-duplication,
and asked for help: I think I will not be able to fix that (fast).

> - Make sure you address as many cosmetic issues as you can before
>   sending a patch.  They only distract from more substantive reviews.

I - very slightly - disagree, and I'd like to add that I addressed exactly
"as many cosmetic issues as" I could. (I agree that these were not enough
though.)
And that is exactly what I did for the first round of the FFmpeg VDPAU patches!

Carl Eugen





More information about the MPlayer-dev-eng mailing list