[FFmpeg-devel] [PATCH 03/12] mdct: remove temporary array in ff_kbd_window_init()

Michael Niedermayer michaelni
Thu Jun 24 03:34:46 CEST 2010


On Thu, Jun 24, 2010 at 01:58:04AM +0100, M?ns Rullg?rd wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > On Thu, Jun 24, 2010 at 01:35:06AM +0100, M?ns Rullg?rd wrote:
> >> Michael Niedermayer <michaelni at gmx.at> writes:
> >> 
> >> > On Thu, Jun 24, 2010 at 12:37:34AM +0100, M?ns Rullg?rd wrote:
> >> >> Michael Niedermayer <michaelni at gmx.at> writes:
> >> >> 
> >> >> > On Wed, Jun 23, 2010 at 06:26:41PM +0100, Mans Rullgard wrote:
> >> >> >> The intermediate values can be stored in the output array, avoiding
> >> >> >> the need for a variable-length array.
> >> >> >
> >> >> > this can write into static arrays and thus introduces a race condition
> >> >> 
> >> >> Quite.  Which would you prefer then, 1) malloc/free or, 2) always
> >> >> allocating a [1024] array (largest size used)?  Option 2 would use 4k
> >> >> (float) or 8k (double) of stack space, which is IMO a bit larger than
> >> >> what's comfortable.  It would of course not change anything compared
> >> >> to current code so should be safe.
> >> >
> >> > as its speed irrelevant init code iam ok with malloc()
> >> 
> >> Using malloc complicates error handling though.  The function
> >> currently returns no status (since it cannot fail), and the callers
> >> obviously do not check it.
> >> 
> >> Since allocating a 1024-element array is fine now, it should be OK
> >> even if hardcoded.  Do you forsee larger sizes being required in the
> >> future?
> >
> > The future is in perpetual motion, iam not capable to say anything
> > with certainity but i feel some remote danger. Maybe some dark plan of the
> > high comiitee or some noble warrior whos heart was not strong enough
> > while walking to close to darkness.
> > One of them might create a spec that requires a larger window.
> 
> Try to be serious for once.  If I read that correctly, you're saying
> you don't know of anything that would need a larger window, and are
> certainly not working on any such code at the moment.  I therefor

yes


> propose setting the max size to 1024 and stating this clearly with a
> comment and a #define.  If we ever need it larger, we can deal with it
> then.

please make it an assert too

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100624/9a37d4fc/attachment.pgp>



More information about the ffmpeg-devel mailing list