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

[Baptiste Coudurier]

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",
you also should know what will happen with mov style w/h CORRECT cropping,
aren't you also mov.c maintainer ?

>> anyway consider that as
>> a bug report 
> 
> ok, then write a proper bugreport

You are aware of the problem now, you have the sample.

>> 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. All bugs are important,
it's amusing how much you can flame gcc about bugs not fixed but how you are
reluctant to fix ffmpeg bugs, and your own bugs sometimes (r_frame_rate
bugs, compute_pkt_fields bugs)

>>> 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, but still a problem
for you.

>>>> 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.

>>>> 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.

> 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.

>>> 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 ?

>> "Also please do not submit patches which contain several unrelated
>> changes. Split them into individual self-contained patches; this makes
>> reviewing them much easier."
>>
>> Are you having problem tracking patches/bugs ?
> 
> yes
> 
> 
>> Where is roundup bug tracker ?
> 
> iam curious about that too, i assume the admin is busy with other things
> maybe i should flame him like you flame me for not spending more of my
> free time dealing with your problems

It's your problem, first you are mjpeg.c maintainer, second you are
ffmpeg project leader and finally Im helpful to send patches to fix bugs
while I should just don't care.

>> Btw, Im still asking for full svn dump.
> 
> yes and you have my full support, but as said i dont have access to the
> svn admin stuff

You have power to request removing of someone's access but not to ask
for an svn dump ?

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