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

Sat Apr 14 02:59:07 CEST 2007

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

> 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

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

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

> 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

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

> but still a problem
> for you.

no thats false

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

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

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

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

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

[...]
> >> 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 ?

i had to threaten root with moving ffmpeg away from mphq before they removed
uotis write access to ffmpeg svn

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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-devel/attachments/20070414/b00af7a1/attachment.pgp>