[Ffmpeg-devel] [PATCH] mjpeg cleanup and again interlaced fix
Sat Apr 14 00:38:48 CEST 2007
Michael Niedermayer wrote:
> On Wed, Apr 11, 2007 at 02:17:14PM +0200, Baptiste Coudurier wrote:
>> 3 patches:
>> - remove useless MpegEncContext.
>> - fix odd field height decoding.
>> - fix some interlaced content.
> what is some?
and any of bottom field first in avi.
>> odd_heigth.mov, this sample is 720x405,
> where is this sample?
mplayerhq, where do you want it to be ?
>> 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, anyway consider that as
a bug report and feel free to fix it the way you want.
> 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? you mean progressive, if so write progressive
>> 1 means encoded from first field, 2 means encoded from second
> 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?
>> 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
Up to you, fix bugs in the code you are maintaining if you don't want to
> sorry but if you cannot clearly describe what your patches do and why then
> they are rejected
> the amount of time i spend with your patches likely exceed he time you spended
> writing them, i feel like iam being throwen random hacks at and then you even
> pester me so i waste even more time looking at the mess again to somehow
> make sense of what exactly the patch really changes while you know exactly
> but dont describe it in a understandable way
Well, you'll just get bug reports for now, if this will save you time,
as you wish.
Btw, quoting ffmpeg-doc.texi:
"All patches posted to ffmpeg-devel will be reviewed, unless they
contain a clear note that the patch is not for SVN."
> 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 ?
"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 ?
Where is roundup bug tracker ?
Btw, Im still asking for full svn dump.
Baptiste COUDURIER GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A. http://www.smartjog.com
Key fingerprint 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312
More information about the ffmpeg-devel