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

[James Almer]

On 5/31/2020 4:11 PM, Andreas Rheinhardt wrote:
> 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.

Yes, that's what i meant. The entire codebase uses flush, drain and EOF
interchangeably in some cases.

> 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).

data != NULL && size == 0 should IMO not be accepted. If we want to
signal that we're feeding the bsf something like timestamps only, we'd
have to find a different way to do it than setting pkt->data to some
bogus value that's not NULL in order to bypass the IS_EMPTY() check.

> 
> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>