[Ffmpeg-devel] [PATCH] mjpeg cleanup and again interlaced fix

[Baptiste Coudurier]

Michael Niedermayer wrote:
> Hi
> 
> On Sat, Apr 14, 2007 at 01:50:50AM +0200, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
>>> Hi
>>>
>>> On Sat, Apr 14, 2007 at 12:38:48AM +0200, Baptiste Coudurier wrote:
>>> [...]
>>>>>> and setting height *= 2 is then
>>>>>> wrong, correct is to set it to container height.
>>>>>> Since height change every field, do that for every field, not only for
>>>>>> first picture.
>>>>> what the 2nd patch seems really to do is 
>>>>> 1. correct a bug where half the init code was only exceuted for width/height
>>>>> changes during the first frame, this worked fine with even interlaced height
>>>>> but with odd interlaced height the height changes after every field and so
>>>>> the init code gets executed after every field but due to the bug just half
>>>>> of it gets executed ...
>>>>> 2. replace frame height= jpeg height*2  by frame height= container height
>>>>> why this is hidden under a million of conditionals is unclear
>>>>> also its unclear what will happen with mov style w/h missuse for croping
>>>> You are maintainer of mjpeg.c, you should know, 
>>> i should know the effects of YOUR patch?
>> you should know "why this is hidden under a million of conditionals is
>> unclear",
> 
> currently the code multiplies height by 2 that has to be hidden because it
> well is wrong for non interlaced content, but setting height to the container
> height like after your patch is a different story, why this is still under
> the set of if() for interlaced content is not clear and thats your code not
> mine ...

It's not my code, I kept original check, and only removed first_picture
and I explained why, I'll repeat in hope to be more clear, height change
for every field with that odd height sample.

>> you also should know what will happen with mov style w/h CORRECT cropping,
>> aren't you also mov.c maintainer ?
> 
> well i think mjpeg after the 2nd patch from you will break with mov
> cropping, which would mean your 2nd patch is buggy

Well, it might break for cropping, but it is IMHO less buggy than
before, patch addresses an issue, and does not address the other issue,
but still height *2 is wrong.

>>>> anyway consider that as
>>>> a bug report 
>>> ok, then write a proper bugreport
>> You are aware of the problem now, you have the sample.
> 
> iam a volunteer, if people dont provide proper bugreports i wont look at
> the bug, and giving me a sample and a patch i dont understand is not
> a bugreport

hey michael Im also a volunteer, I consider trying to fix the bug myself
to avoid you looking at it better than simply bugreporting, now I don't
understand why you always answer with aggressivity and without any trust
at all, yes humans makes mistakes, and if the code is still buggy for
another other sample I'll try to fix it, you know the story, look at
mov/asf/avi workarounds/bugs.

>>>> and feel free to fix it the way you want.
>>> i dont have time to fix this (for me quite unimportant bug)
>> That's not an acceptable answer. 
> 
> my awnser doesnt has to be accpetable to you, your awnsers also arent
> acceptable to me ...
> 
> 
>> All bugs are important,
> 
> then why dont you fix H264-PAFF ?

Well, honestly because I don't have the necessary skills, and I always
try to fix bugs, even when it's not my code or code I don't maintain
(adcpm swf decoding, swf de/muxer, gif) you are being rude saying that.

>> it's amusing how much you can flame gcc about bugs not fixed but how you are
>> reluctant to fix ffmpeg bugs, 
> 
> i primarely flame gcc for pretending that their code is not buggy and for
> closing bugs without fixing them or rather changeing the specification (docs)
> instead of their code and similar ...
> 
> 
>> and your own bugs sometimes (r_frame_rate
>> bugs, compute_pkt_fields bugs)
> 
> i dont know what you are talking about, r_frame_rate is a guess it CANNOT
> be always correct even less so in cases where even humans dissagree which
> framerate is the correct one
> and the docs clearly say that r_frame_rate is a guessed value

Yes, but you must admit that changing it is oftenly introducing another
weird behaviour/bug with another sample. I reported some bug already,
and I'm still looking for a good solution.

Now problem is that I think you are not acting as part of a team and you
don't hint us with your thinkings which are oftenly good and useful, and
you know the whole ffmpeg code better than anyone else. Sometimes you do
that's right, but lately at least with me you are really more answering
with "rejected".

>>>>> what the 3rd patch does is still unclear to me ...
>>>>>
>>>>>
>>>>>> ODML specs specify the use of AVI1 tag, and meaning of polarity (0 for
>>>>> what does ODML have to do with mjpeg? is this series of patches related to
>>>>> mjpeg in avi? the sample file whos location we dont knoe is a .mov not avi
>>>> RTFSpecs. ODML (MJPEG DIB)
>>>>
>>>>>> prog, 
>>>>> prog? you mean progressive, if so write progressive
>>>> why ?
>>> well, if you want your patch in svn it has to pass review and that means i
>>> must understand what it does and why, if i dont, i wont approve the patch
>>> now if you write random brain dump with a bunch of "RTFS" even though you
>>> either know what the patch does but dont tell or you dumped random hacks 
>>> on ffmpeg-dev either way patch rejected
>> It's in my tree and that means one less bug for me, 
> 
> considering that the 2nd patch introduces a bug i dont think so

Well maybe but I still think that it is better than current situation.

>> but still a problem
>> for you.
> 
> no thats false

Seriously you don't consider having bugs in ffmpeg a problem for you ?
Well honestly ffmpeg bugs are a problem for me, considering myself as a
part of the development team.

>>>>>> 1 means encoded from first field, 2 means encoded from second
>>>>>> field), 
>>>>> so you speak about a variable somewhere in the ODML-avi container spec
>>>>> which stores a progressive/interlacaed and temporal field order or is it
>>>>> rather spatial order flag? hows that related to mjpeg and how to the patches?
>>>> RTFSpecs again.
>>> i already said i dont have the time to do YOUR work, the person submitting a
>>> patch is the one who has to explain what and why not point at 2 several 100
>>> page specs with some not understandable comments
>> I explained what it does, now if YOU don't understand that's your
>> problem, and you should at least download the sample and try it. That's
>> what I do for the bugs related to the files I maintain, and every
>> maintainer should do the same.
> 
> i maintain a lot and simply dont have the time to do this with every file
> someone mentions in some mail

Yes and I think everyone is thankful, at least I am even if Im oftenly
fighting. Now I react differently concerning bugs, but don't you let
people helping you, you always review commit logs so if sometimes seems
weird you'll mention it and it will be fixed, it avoids to send patch
for one and reviewing for you, but that's right that might lead to more
bugs depending on the developper.

> i think you know what you patch does and why and its not unreasonable to
> expect you to explain this to me instead of asking me to go over a few 100
> pages of manuals ...

Of course I know, but Im quite exhausted feeling no trust whatsoever and
always nitpicking, and I think Im not the only one developer feeling
that. If you honestly think Im throwing garbage at you, then Im sorry
because you have no idea about how much time I spend verifying cosmetics
with svn diff, and taking care of explaining, now you never understand,
what can I do ? Spend more time ? Well Im also doing a lot of things.

>>>>>> and only set polarity when first field is being decoded, use of
>>>>>> new second_field in context.
>>>>> this makes no sense, iam i supposed to read the jpeg and odml specs and then
>>>>> reverse engeneer what the patch does? your comments are as usefull as 
>>>>> /dev/random
>>>> Up to you, fix bugs in the code you are maintaining if you don't want to
>>>> review patches.
>>> iam not aware of any proper bugreport 
>> You are just lying.
> 
> no its the truth that iam not aware of any proper bugreport related to this
> if you are aware of one why dont you provide a link?
> also you cant expect me to remember every bugreport ...

I consider my mail as a bugreport, you are asking me to spend more time
to something you are now aware about, you can just mark the mail as
important/to fix and then come back later. If I do some efforts to
please you, I think I deserve some effort in return, Im putting a lot of
myself into ffmpeg too.

> either way, the amount of time i spend in idiotic flamewars with you is
> unaccpetable, every time i ask a question or reject a buggy patch from you
> you start a silly fight

Yes, because I really think that's worth it for both of us.

>>> also i did review your patches but you dont awnser questions, provide no
>>> hint on what the patch does and i dont have a whole day to figure out the
>>> things you refuse to tell me
>> Im not really inclined to answer voluntary aggressive and evasive questions.
> 
> it was not my intent to be aggressive, so if any question seemed aggressive
> iam sorry it was not intended that way iam not a native english speaker ...

So am I.

>>>>> its not even clear which of your comments relates to which of the patches
>>>>> that is without looking at the patches ...
>>>>>
>>>>> also the 3rd patch contains cosmetic changes!
>>>>>
>>>>> and each patch should be in a seperate mail
>>>> Where is it mentioned ?
>>> maybe it isnt in the docs, i will change this, thanks for noticing
>> Changing policy at your own will without asking other
>> reviewers/developpers ? Is that democracy or tyranny ?
> 
> you are free to start a vote, i will revert the change if the majority
> wants it

Honestly problem is that you too oftenly do that, and people is not
caring at all, look at how many people answer or discuss about sensitive
topics, so people might think but not say because they don't want to
fight/flame.

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312