On 7/30/20, Andreas Rheinhardt <andreas.rheinha...@gmail.com> wrote:
> Paul B Mahol:
>>
>> +static int cfhd_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>> + const AVFrame *frame, int *got_packet)
>> +{
>> + CFHDEncContext *s = avctx->priv_data;
>> + PutByteContext *pby = &s->pby;
>> + PutBitContext *pb = &s->pb;
>> + const Codebook *const cb = s->cb;
>> + const Runbook *const rb = s->rb;
>
> Why are these part of the context and not simply on the stack although
> they are reinitialized every time they are used?
Because I'm lazy typing & every single time.
>
>> + unsigned pos;
>> + int ret = 0;
>
>
>
>>
>> + ret = ff_alloc_packet2(avctx, pkt, 6 * avctx->width * avctx->height,
>> 0);
>> + if (ret < 0)
>> + return ret;
>> +
>
> You are writing quite a lot of stuff whose length is independent of the
> dimensions (E.g. you are writing 60 + 4 * s->planes before you enter the
> big for-loop). Is it possible that the allocated size might be
> insufficient for small dimensions (when the allocated packet is small,
> yet the constant part might not be)? (Or is there some check that I
> missed that rules out small dimensions?) And shouldn't you explicitly
> check whether your buffer was too small?
Fixed.
>
>> + bytestream2_init_writer(pby, pkt->data, pkt->size);
>> +
>
>
>>
>> + avpriv_align_put_bits(pb);
>
> Unnecessary, flush_put_bits() already fills the bitstream with zeroes.
Fixed.
>
>> + flush_put_bits(pb);
>> + bytestream2_skip_p(pby, put_bits_count(pb) >> 3);
>> + while (bytestream2_tell_p(pby) & 3)
>> + bytestream2_put_byte(pby, 0);
>
> It seems to me that this loop can be an infinite loop if the buffer
> turned out too small (because you are using the safe version of the
> bytestream2 API and if the packet size is not divisible by four (happens
> if and only if avctx->width and avctx->height are odd), then you will
> not advance to an element whose distance from the beginning is divisible
> by four).
Fixed.
>>
>> + pkt->size = bytestream2_tell_p(pby);
>> + pkt->flags |= AV_PKT_FLAG_KEY;
>> +
>> + bytestream2_seek_p(pby, 8, SEEK_SET);
>> + for (int i = 0; i < s->planes; i++)
>> + bytestream2_put_be32(pby, s->plane[i].size);
>> +
>> + av_assert0((pkt->size & 3) == 0);
>> +
>
> If your buffer turned out to be too small, the above assert might be
> triggered.
Removed. Anyway if buffer is too small it will assert in another code.
>
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
