[FFmpeg-cvslog] random thoughts about refactoring
Diego Biurrun
diego
Sun Jan 10 19:14:27 CET 2010
On Sun, Jan 10, 2010 at 05:42:24PM +0100, Michael Niedermayer wrote:
> On Sun, Jan 10, 2010 at 12:44:24PM +0100, Diego Biurrun wrote:
> > On Fri, Jan 08, 2010 at 02:28:16AM +0100, Michael Niedermayer wrote:
> > > On Fri, Jan 08, 2010 at 01:20:03AM +0100, Diego Biurrun wrote:
> > > > On Wed, Jan 06, 2010 at 06:38:14PM +0100, Michael Niedermayer wrote:
> > > > > On Tue, Jan 05, 2010 at 11:08:22PM +0100, Diego Biurrun wrote:
> > > > > > On Tue, Jan 05, 2010 at 08:43:51PM +0100, Michael Niedermayer wrote:
> > > > > > > On Tue, Jan 05, 2010 at 11:29:46AM +0100, Diego Biurrun wrote:
> > > > > > > > On Tue, Jan 05, 2010 at 03:20:47AM +0100, Michael Niedermayer wrote:
> > > > > > > > > On Mon, Jan 04, 2010 at 08:23:23PM +0100, Diego Biurrun wrote:
> [...]
> > > > > > > > > i still think copy and pasting the 4 hunks into patch is going to be the
> > > > > > > > > quicker way to fix this, thats not disputing that you eventually will come
> > > > > > > > > up with a convertion to proprer comments just that the work seems out of
> > > > > > > > > proportion to the purely cosmetic gain
> > > > > > > >
> > > > > > > > What is out of proportion is the ensuing discussion. What is
> > > > > > > > disheartening is the lack of motivation to actually fix the current
> > > > > > > > mess.
> > > > > > >
> > > > > > > My fix is to revert this. You refuse to do that. I dont really mind
> > > > > > > an alternative fix but i cannot fix what you precive as mess because
> > > > > > > i do not precive these //printf("some error condition") as mess.
> > > > > > > To me a note on toilet paper is as good as one on gold framed under glass
> > > > > >
> > > > > > Crufty code is not good, it will spread, do you know the broken window
> > > > > > theory?
> > > > >
> > > > > i didnt but i dont see how this is related nor do i see reason to belive that
> > > > > this theory has any scientific truth to it, it sounds alot more like mixing
> > > > > correlation with causation.
> > > >
> > > > I just have to look at my desk to confirm the theory. Ever since I
> > > > stopped keeping it tidy a few months ago, things keep piling up in
> > > > ways previously thought impossible.
> > > >
> > > > Or try sharing a kitchen with a few people for some time. Once you
> > > > lower the standards, everybody will start being messy.
> > > >
> > > > That's not saying that it will apply to all groups of people at any
> > > > time, but it will in most common cases (and I think ours is one).
> > > > Sociology works this way, it's impossible to conduct experiments with
> > > > the same rigor as in physics there...
> > >
> > > hmm, break your window and dont fix it, lets see if this will cause any
> > > of your neighbors windows to allso break and stay broken
> > > simple scientific test ...
> >
> > But it's a simple scientific test of a different thing. You cannot just
> > change multiple surrounding factors and hope for the results to stay
> > meaningful.
> >
> > Alternative experiment: Let's stop policing top-posting and offtopic
> > posting on ffmpeg-devel. Then wait for a year and see how habits
> > have changed...
>
> yes if you don fix ANY broken window you will of course have the number of
> broken windows increase. What the original claim was about though was that
> not fixing a subset of broken windows will by "magic" affect windows
> outside this set.
I don't see us make any progress discussing sociology. Let's drop the
subject.
> > > > > > There are people here who know H.264 well enough to lend
> > > > > > a helping hand in theory, but in practice the implementation in lavc is
> > > > > > just too impenetrable. h264.c is around 10.000 lines if you count svq3.c
> > > > > > which it directly includes!
> > > > >
> > > > > >
> > > > > > Also, our H.264 decoder is not a speed demon.
> > > > >
> > > > > That seems to depend on CPU and situation, gcc making poor inlining choices
> > > > > and overflowing the available caches is not exactly the codes fault ...
> > > >
> > > > Carl Eugen was the first person I have ever heard claim that our
> > > > H.264 decoder was faster than anything, ever...
> > >
> > > Ive never tried CoreAVC, but i can assure you the reference implementation
> > > is much much slower
> >
> > You are referring to the H.264 reference implementation?
> >
> > Jason mentioned two new H.264 decoders on IRC that are even faster than
> > CoreAVC...
>
> Why is this information not posted on the mailing list ?
I don't see much gain from this. What is the news factor? More H.264
decoders that whip FFmpeg's ass? Our decoder is slow, everybody knows
this already...
> > > Also our mpeg1/2 decoder could be improved by mere throwing all non mpeg1/2
> > > code out. i dont remember how much faster that made it but it was significant
> >
> > This would likely be worthwhile.
> >
> > How about making 2010 the year of performance improvements?
>
> Prerequesite to that would be keeping h264 discussions where the only person
> who works on our decoder (yeah thats me) sees them
>
> Its really sad, how often do i have to repeat that i cannot do all the work
> alone. Now if people refuse to send cleanly split patches, could they at least
> _please_ keep their h264 discussions on ffmpeg-dev!
People just talk about whatever tickles their fancy on IRC. You cannot
make them stop. Some people are mailing list types, others prefer IRC.
It's just the way it is.
> > > > > > I tried refactoring parts of it some time ago but failed horribly.
> > > > >
> > > > > Thats because you are lacking knowledge of h264 ...
> > > >
> > > > I successfully split off svq3.c from h264.c. No knowledge of H.264
> > > > internals was necessary for this task. You rejected the patch because
> > > > of minor speed regressions.
> > > >
> > > > > > Since you have declared that even the most minor speed regression
> > > > > > is unacceptable, I don't think anybody will muster the courage to
> > > > > > try in the future...
> > > > >
> > > > > did i?
> > > > > i think i tend to insist on well split patches and on benchmarks on
> > > > > the individual parts. That is to weed out changes that worsen things
> > > >
> > > > I understood this to be the case after my failed svq3.c split. As said,
> > > > it's been a recurring topic on IRC and I'm not the only one to think this
> > > > is the state of things.
> > >
> > > ill look at that patch again (cant comment without looking at what was done
> > > and what the problem was)
> >
> > There were two patches from me that were rejected: one for splitting off
> > svq3.c from h264.c and another for splitting off the code used by both
> > the decoder and the parser.
> >
> > I'd appreciate if you could revisit both. I can look into refreshing
> > them when I have time again.
>
> I searched but failed to find them, could you tell me what the subject was?
The SVQ3/H.264 split:
http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2008-November/056579.html
[FFmpeg-devel] [RFC] H.264/SQV3 separation: h264data.h
I made a typo in the subject (SVQ3 vs. SQV3), that's likely the reason
you could not find it.
The H.264 parser and decoder common code split:
http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2009-June/071247.html
[FFmpeg-devel] [PATCH] refactor H.264 decoder/parser common code
You said this was not an ideal split. IMNSHO the way it is now - one
monolithic file - is even worse...
Diego
More information about the ffmpeg-cvslog
mailing list