On 2012-06-14 16:36:44 +0200, Christophe Gisquet wrote: > Hi, > > 2012/6/14 Mans Rullgard <[email protected]>: > > static void guess_mv(MpegEncContext *s) > > { > > - uint8_t fixed[s->mb_stride * s->mb_height]; > > + uint8_t *fixed = s->fixed; > > What about setting s->fixed to NULL when initializing, and only > setting s->fixed to something here, like (just an example): > uint8_t *fixed; > if (!s->fixed) > { > FF_ALLOCZ_OR_GOTO(s->avctx, s->fixed, mb_array_size * sizeof(uint8_t), > fail); > } > fixed = s->fixed; > [...] > fail: > /* should probably return error here */ > > That way, no unnecessary allocation (even though it is small) is done > if ER does not need to be performed. I know error handling and > probably style are not correct in the above code snippet, so just take > this as an example. > > All in all, if the above is too much of a bother, it is better to have > your patch applied than nothing.
I think the allocation small enough to not care since returning from error resilience in the case of an allocation error is non-trivial. > > > + /* quantization tables */ > > + FF_ALLOCZ_OR_GOTO(s->avctx, s->cplx_tab, > > + mb_array_size * sizeof(float), fail); > > + FF_ALLOCZ_OR_GOTO(s->avctx, s->bits_tab, > > + mb_array_size * sizeof(float), fail); > > They do not look like quantization tables to me, but encoder image > analysis data for rate control. they are used during adaptive quantization, so /* image analysis data for adaptive quantization */ might be a better comment > Also, I guess FF_ALLOC_OR_GOTO can be used (no need to zero such an array). same is true for s->fixed, all entries are set before use. Janne _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
