[FFmpeg-cvslog] r23977 - trunk/libavcodec/rl2.c

Michael Niedermayer michaelni
Thu Jul 8 20:56:12 CEST 2010


On Thu, Jul 08, 2010 at 08:38:09PM +0200, Diego Biurrun wrote:
> On Thu, Jul 08, 2010 at 07:44:53PM +0200, Michael Niedermayer wrote:
> > On Thu, Jul 08, 2010 at 03:28:23PM +0200, Diego Biurrun wrote:
> > > On Fri, Jul 02, 2010 at 02:15:46PM +0200, Michael Niedermayer wrote:
> > > > On Fri, Jul 02, 2010 at 01:37:54PM +0200, diego wrote:
> > > > > 
> > > > > Log:
> > > > > Remove two more non-existing stray Doxygen function arguments.
> > > > 
> > > > wrong as well
> > > 
> > > Yes, there is still a bug, one function parameter remains undocumented.
> > > Doxygen generates the following warning for it:
> > > 
> > > /usr/src/ffmpeg/trunk/libavcodec/rl2.c:172: Warning: The following parameters of rl2_decode_frame(AVCodecContext *avctx, void *data, int *data_size, AVPacket *avpkt) are not documented:
> > >   parameter 'avpkt'
> > > 
> > > I removed the documentation for two paramaters which no longer exist
> > > in the code, and this change is correct.  Documenting the remaining
> > > parameter is a separate issue.
> > 
> > no, its not seperate, the 2 parameters where merged into avpkt.
> > the corresponding change for the doxy is to merge them too, not to remove
> > them and wait for someone else to fix it up
> > 
> > you can just revert the change or fix it properly but spliting a move
> > or merge into a remove you do and add you dont do is not ok
> > 
> > the warnings where showing a problem, that is a forgotten update of the
> > doxy after a change to the code. Your change does not correct this problem
> > it is thus per definition not a solution for this problem.
> > 
> > When a warning indicates a problem then this problem must be solved and
> > until this is done the warning must stay.
> > 
> > When a warning does not indicate a problem the warning must be removed with
> > a minimum of negative sideefects.
> 
> There were two warnings: one for the undocumented parameter, one for the
> removed parameters.  I did not have time to fix the first one, just the
> second one.  There is still a warning that points at the first issue, so
> nothing is being hidden.

they are not seperate warnings, they together indicate the problem.
Your change makes it strictly worse. please revert it or close your
svn write account whichever you prefer.

If you dont understand the code, try to understand a car analogy
warning your tires are not on your car
warning your car has no tires
warning you dont have a functioning car

you dont improve this by throwing the car and tires away to reach just:

warning you dont have a functioning car



> 
> In an ideal world I would have fixed both issues in one fell swoop, but
> in the sad reality we have to live in we sometimes have to accept less
> than ideal solutions.

there is one issue not 2 issues. missing documentation is also not
5000 issues of missing letters


> 
> My commit is an incremental improvement with no ill sideeffects.  Now the

iam getting a headache from it


> doxygen is only incomplete instead of lying and being incomplete.  There
> is no reason to reject such incremental improvements flat out just because
> a perfect solution exists.
> 
> The perfect solution has higher cost and I am not willing to pay for it
> with my time right now.  Please respect this decision, which is mine to
> make.
> 

> Since reverting my commit would introduce a bug without fixing another,

a warning correctly indicating an existing bug is no bug


> I am unwilling to do it.  The offending commit that introduced the issue
> in the first place is r18351:

perfectly fine, then close your account, iam sick of these discussions


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20100708/a968d621/attachment.pgp>



More information about the ffmpeg-cvslog mailing list