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

[Michael Niedermayer]

On Thu, Jul 08, 2010 at 09:10:39PM +0100, M?ns Rullg?rd wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > On Thu, Jul 08, 2010 at 08:30:26PM +0100, M?ns Rullg?rd wrote:
> >> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> >> 
> >> > On Thu, Jul 08, 2010 at 08:12:02PM +0100, M?ns Rullg?rd wrote:
> >> >> Diego, and everybody else here, is doing his best to help improve
> >> >> things.  The rest of us here have jobs and/or school to share our time
> >> >> with, and try to get the most out of the time we put into FFmpeg.  All
> >> >> too often you have rejected patches for no other reason than not being
> >> >> provably perfect, even though they were without question an
> >> >> improvement.
> >> >
> >> > Well, I do think that more often a "definitely better than what we have"
> >> > would be a good reason to ok a patch on its own.
> >> > However I was a bit annoyed about parts of the patch as well, because it
> >> > seemed remove documentation. Even when it does not apply 100% anymore,
> >> > just removing it in my opinion is far from an improvement, and would have
> >> > warrented at least asking about it before....
> >> 
> >> I do not see how comments talking about parameters that no longer
> >> exist are in the slightest bit helpful.
> >
> > they still exist in avpkt and the function will crash if they arent set
> 
> How is the reader of the documentation supposed to divine that?

he can see in the documentation that there is a buf and buf_size and that
there are no such parameters. He thus will know that there is something
mismatching, he further might spot that avpkt is as only parameter not
documented and that it contains a size and data member.

he could grep for buf/buf_size in APIChanges finding:
2009-04-07 - r18351 - lavc 52.23.0 - avcodec_decode_video/audio/subtitle
  The old decoding functions are deprecated, all new code should use the
  new functions avcodec_decode_video2(), avcodec_decode_audio3() and
  avcodec_decode_subtitle2(). These new functions take an AVPacket *pkt
  argument instead of a const uint8_t *buf / int buf_size pair.

he could check svnlog/blame to find out when the mismatching was introduced
and where the parameters went

alternatively with buf/buf_size removed from the docs the mismatch is gone
it looks like avpkt has never been documented
the user will probably stillfind out that theres something missing but
he has fewer hints and options.

But its not just the user.
Some C developer who wants to actually fix the shortcoming of the doxy
will with the buf/buf_size->APIChanges know alot more than without that
chain of reasoning.
Especially the question of which fields of avpkt need to be set and which
not is much easier to awnser if you know it was the result of a merge and
not a previously undocumented field. one can in the first case look at
the diff and APIChanges in the later one might not even try to find it
and rather reverse engeneer code...


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- 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/bad2eff7/attachment.pgp>