On 16/08/2012 3:29 PM, Kostya Shishkov wrote:
>> +static const int pred_order[5] = {
>> + PRED_LEFT, PRED_MEDIAN, PRED_MEDIAN, PRED_NONE, PRED_GRADIENT
>> +};
> A comment would be nice, otherwise it looks "I'll guess one of four prediction
> methods with five guesses. What, is it really not median?"
Perhaps line breaks too? To match the other declarations.
> + if (count) {
> + put_bits(&pb, 32 - count, 0);
> + }
Useless {}.
> + for (i = 0; i < 256; i++) {
> + if (i == symbol)
> + bytestream2_put_byte(pb, 0);
> + else
> + bytestream2_put_byte(pb, 0xFF);
> + }
> +
Could use bitwise operators to avoid a branch, but the speed gain
would be negligible.
How MiNi are you?
>> + slice_buffer = av_malloc(width * height + FF_INPUT_BUFFER_PADDING_SIZE);
>> +
>> + if (!slice_buffer) {
>> + av_log(avctx, AV_LOG_ERROR, "Cannot allocate temporary buffer
>> 1.\n");
>> + return AVERROR(ENOMEM);
>> + }
>
> Why not allocate it once during init?
As I said in my previous review, once you move this to init, you may want to
simply call encode_close from inside encode_init upon failure, rather than
stacking multiple freep().
Also, the way this alloc currently is, it will leak when encoding fails
for any number of reasons.
- Derek
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
