[Ffmpeg-cvslog] r8204 - trunk/libavformat/mov.c

[Michael Niedermayer]

Hi

On Sat, Mar 03, 2007 at 10:44:10PM +0100, Baptiste Coudurier wrote:
> Michael Niedermayer wrote:
> > Hi
> > 
> > On Sat, Mar 03, 2007 at 04:00:08PM -0500, The Wanderer wrote:
> >> Michael Niedermayer wrote:
> >>
> >>> Hi
> >>>
> >>> On Sat, Mar 03, 2007 at 08:48:35PM +0100, Baptiste Coudurier wrote:
> >>>> You going to revert it for 2 lines of fourccs even more when those
> >>>> 2 lines are mentioned in the commit log ?
> >>> yes
> >> <snip>
> >>
> >>> btw, i didnt even realize that its 2 lines and that exactly is the
> >>> problem with such commits you never know what really has been changed
> >>> if cosmetics get mixed with functional changes
> >> I read through the patch looking for the change described, to make sure
> >> I wasn't incorrect before posting, and I was unable to even tell with
> >> certainty (in a reasonable amount of time) which lines contained the new
> >> tags - because the table was also being rearranged by the same commit.
> >> As a result, I was unable to be sure whether or not a non-cosmetic
> >> change had been made.
> >>
> >> This is part of the reason for the rule against mixing functional and
> >> cosmetic changes, and part the reason for the rule against mixing
> >> unrelated changes: it makes reading the patch to find a particular
> >> change harder. (It also means that if one change ever needs to be
> >> reverted, it is significantly harder to do so without also reverting any
> >> others which were in the same patch.)
> 
> Like if you would ever patch something buggy. 

no the wanderer doesnt normally but people who do have difficulty makeing
sense of such a change too (i for one had to play with diff and sort to
find the WMV3 change)

please look at the diff yourself and then ask yourself if you could figure
out what was changed with the diff and the commit log message


> Let's talk about
> meritocracy a bit, like someone suggested ?

we can if you like, preferably on ffmpeg-dev though, its a little off topic
here


> 
> > ive fixed the mess, ive also found that WMV3 was removed in the change
> > which the commit message had not mentioned and baptiste apparently forgot
> > 
> > baptiste please be more carefull in the future
> 
> Well, for sure, I won't clean any mess in the future. Yes im quite
> upset, and tired.

it was not my intent to piss you off, you are a valuable developer and
your cleanups are welcome, just this one was done in an unaccpetable way
ive fixed it not you so why are you angry?


> 
> > btw why was WMV3 removed?
> 
> cleanup, wmv3 is not needed since vc1 fourcc will pick up CODEC_ID_VC1
> for demuxing first, and never wmv3.

but for muxing WMV3 will fail to find the tag while if it where in the
table it would succeed, at least that seems to be the case, the table is
used on the muxer side too
of course it might be wrong to use that tag for wmv3 i dont know but
its not a cosmetical change if a entry is removed from a table


> 
> Note that technically, adding those codec FOURCCs had absolutely no
> effect on the code, since BE codec id would have been chosen first.

same thing as above

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20070304/16fa302e/attachment.pgp>