On Tue, Apr 26, 2011 at 06:24:55AM +0000, Ronald S. Bultje wrote:
> 
> On Mon, Apr 25, 2011 at 10:46 PM, Diego Biurrun <[email protected]> wrote:
> > On Tue, Apr 26, 2011 at 12:27:48AM +0200, Janne Grunau wrote:
> >> --- a/libswscale/utils.c
> >> +++ b/libswscale/utils.c
> >> @@ -817,11 +817,11 @@ int sws_init_context(SwsContext *c, SwsFilter 
> >> *srcFilter, SwsFilter *dstFilter)
> >>
> >>      if (!isSupportedIn(srcFormat)) {
> >> -        av_log(NULL, AV_LOG_ERROR, "swScaler: %s is not supported as 
> >> input pixel format\n", sws_format_name(srcFormat));
> >> +        av_log(c, AV_LOG_ERROR, "%s is not supported as input pixel 
> >> format\n", sws_format_name(srcFormat));
> >
> > extra good karma for breaking long lines
> 
> We had a rule for this:
> A) no new patches necessary for these kind of comments, and

Sure :)

> B) you can fix it yourself.

And I do it, witness the patches I sent for AVX and the DTS encoder...

> Let's try to keep stuff moving, I see a lot of nitpick-style comments
> again and they're not always as helpful as they're intended...

But only comments can keep us moving forwards.  When I see a function
with 17 (!) parameters, I cannot help but make a remark.  This does
not imply that I expect the poor soul that made a change to that
monster to singlehandedly refactor it.

Nor do I expect all or any of my minor comments to be implemented, but
I believe that every little bit helps, so I'm always happy when people
do implement minor comments, which also happens.  I read patches to
learn from them, sometimes I think that I might as well read them in
the editor and comment here and there ...

I may be allowed to mention here that I am currently into the third day
of a refactoring avalanche that started from me noticing a warning due
to wrong printf specifiers, which led to Justing suggesting to eliminate
int_fast types, which led to Mans suggesting to make more fine-grained
changes which is maybe leading to implementing sizeof trickery...

Did I reach my initial goal?  Sort of.  Am I complaining?  No.  Is it
improving Libav?  Yes.

Maybe we should just classify our review comments with [H] [M] [L] tags
for high, medium and low priority.  I already try to do that somewhat
by talking about nits and karma bonus points...

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to