[FFmpeg-devel] [PATCH 3/3] avcodec/packet: change public function and struct size parameter types to size_t

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Sun May 31 22:11:33 EEST 2020


James Almer:
> On 5/31/2020 2:58 PM, Andreas Rheinhardt wrote:>> (Anyway, when this function is switched to size_t,
>> the correct error would be AVERROR(ERANGE). It is actually already the
>> correct error in case size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE.)
>> 3. That's unfortunately only a tiny fraction of the work to do before
>> this switch can happen:
>> a) libavcodec/mjpega_dump_header_bsf.c contains the following line:
>>      for (i = 0; i < in->size - 1; i++) {
>> If in->size == 0, then this will not have the desired effect, because
>> the subtraction now wraps around. There are probably more places like
>> this (and overflow checks that don't work as expected any more). We
>> would need to find them all and port them.
> 
> Empty packets are considered flush packets, right? Guess we should be
> rejecting packets where pkt->size == 0 && pkt->data != NULL in
> av_bsf_send_packet(), same as we do in avcodec_send_packet(). I'll send
> a patch for that later.
> 
The only way one can signal flushing to a bsf is by using av_bsf_flush.
Empty packets (data == NULL and no side_data) are considered eof.
Packets with data != NULL, size = 0 and no side data are currently
considered normal packets (a possible use-case would be to send some
timestamps to the bsf, although no bsf currently makes use of this; but
who knows what needs a future bsf has).

Also even rejecting such empty packets in av_bsf_send_packet wouldn't
completely solve the problem because a user might send a side-data only
packet. So the check would either be switched to i + 1 < in->size (is
this really slower than i < in->size - 1?) or a check has to be added
before the loop. Also notice that the 1 should actually be 3, see
https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/263632.html.

- Andreas


More information about the ffmpeg-devel mailing list