Janne Grunau <janne-li...@jannau.net> writes:
> On 2012-06-14 16:36:44 +0200, Christophe Gisquet wrote:
>> Hi,
>>
>> 2012/6/14 Mans Rullgard <m...@mansr.com>:
>> > 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
Or drop the comment entirely. It doesn't really add any information.
>> 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.
Speaking of which, the name "fixed" isn't exactly descriptive.
--
Måns Rullgård
m...@mansr.com
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
