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

[Diego Biurrun]

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.

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.

My commit is an incremental improvement with no ill sideeffects.  Now the
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,
I am unwilling to do it.  The offending commit that introduced the issue
in the first place is r18351:

------------------------------------------------------------------------
r18351 | rbultje | 2009-04-07 17:59:50 +0200 (Tue, 07 Apr 2009) | 9 lines

Implement avcodec_decode_video2(), _audio3() and _subtitle2() which takes an
AVPacket argument rather than a const uint8_t *buf + int buf_size. This allows
passing of packet-specific flags from demuxer to decoder, such as the keyframe
flag, which appears necessary to playback corePNG P-frames.

Patch by Thilo Borgmann thilo.borgmann googlemail com, see also the thread
"Google Summer of Code participation" on the mailinglist.
------------------------------------------------------------------------

If the burden of fixing the bug has to be placed on somebody, the person
who introduced it would seem like a better choice than myself.

Diego