[Ffmpeg-devel] Re: [Ffmpeg-cvslog] r6161 - in trunk: libavcodec/dv.c libavcodec/dvdata.h libavformat/avformat.h libavformat/dv.c tests/ffmpeg.regression.ref tests/libav.regression.ref tests/rotozoom.regression.ref

Roman Shaposhnik rvs
Tue Sep 5 03:27:29 CEST 2006


Hi

First of all -- is it really a good practice to follow-up
on fmpeg-cvslog instead of ffmpeg-devel ? I don't like
littering hence my reply to -devel.

On Mon, 2006-09-04 at 12:46 +0200, Michael Niedermayer wrote:
> [...]
> > +enum dv_pack_type {
> > +     dv_header525     = 0x3f, /* see dv_write_pack for important details on */
> > +     dv_header625     = 0xbf, /* these two packs */
> 
> not doxygen compatible comment
 
  Good catch. I'll fix it.

> avformat.h is a public header, changes to it should be disscussed on
> ffmpeg-dev (even more so as you are not the maintainer of it)
> such static inline functions are also not a good idea in a public header
> because they make the user app unnecesarily depend on the implementation
> and thus lead to binary compatibility issues, furthermore these functions
> are used just by the dv code so they really dont belong to avformat.h
> and does the usage of these functions even merit that they are static inline?
 
  I would expect fifo_peek to benefit *a lot* by being a static inline,
since its being used in a pretty hot loop. fifo_drain surely can be 
a plain function without losing much performance. What would you like
me to do with these functions ? If I move them out of avformat.h
completely (that is -- there won't be even declararions of them) then
next time somebody changes internal implementation of the FifoBuffer
my private versions of fifo_peek and fifo_drain will be screwed -- not
a good idea as far as I can tell. Please advise.

I understand your your comment about making changes to the portions of
the ffmpeg which are maintained by others, yet, I haven't seen such a
policy explicitly stated anywhere. Thanks for making me a poster child
for stating the policy. btw, I've got a question: do I have a right to
scream and shout next time somebody makes a [trivial] change to dv.c ? 

> additionally, expect to loose your svn write account if you add such stuff
> to non dv specific code the next time without discussion on ffmpeg-dev.
> you have done this several times by now ...

  Michael, this is not nice. First of all -- please tell me exactly
when was the last time I done so ? Please be specific -- otherwise
you sound like a bickering asshole. Don't get me wrong though -- I do
understand all the technical points you've made. I do actually
agree with most of them but what I don't agree with is the attitude
you've demonstrated towards my person on a number of occasions now.
And what is especially irritating is the fact that you're not even
capable of simply saying that you don't like me (or whatever) and
you don't want me on this project. If I irritate you so much -- just
go ahead and say so. 

   I hope that everything I've written above is not true and we simply
have a gross miscommunication going on between us. But it is really
hard to convince myself when I've seen a much weaker techincal commits 
going in without you making a big deal out of them (no doxygen
comments, changing of the stuff maintained by others, etc.). And I've 
seen downright broken stuff going in, yet the first time I've seen
this line: "additionally, expect to loose your svn write account" it 
was directed at me.
  
  I wish I had a chance of pulling Theo de Raat on you by forking 
ffmpeg -- I can't. You're the capable one. Enjoy being smarter 
while it lasts. It won't last forever.

> could you explain why the regression checksums change, your commit message
> makes the change sound like a pure restructuring which wouldnt change the
> output?

  Michael, do you really want to know or are you just fsck'ing up with
me trying to see whether I was stupid enough to commit a change without
understanding why it changed the regression checksums ?I can not really
tell the difference, you know. 

  Anyway, here's the reason: There was a tiny bug that led to every DV
stream we've generated having the first two frames labeled as 
"frame 0". Now the second frame gets properly tagged as "frame 1".
That's the only difference visible to the regression testing. Not
visible to the regression testing are things like aspect ratio now
being recorded properly.

  Anyway, here's the bottom line -- if I'm still on the project after 
you finish up this line please answer the question about avformat.h
so that I can have a single commit fixing all the issues you've
mentioned.

Thanks,
Roman.





More information about the ffmpeg-devel mailing list