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

[Andreas Rheinhardt]

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.
> 
Bogus value? You seem to believe that pkt->data just points to somewhere
in memory; in a valid packet it points to a buffer of size pkt->size +
AV_INPUT_BUFFER_PADDING_SIZE (even when pkt->size == 0).
av_packet_make_refcounted creates such packets if the input packet
doesn't have data (and the h264_mp4toannexb bsf currently relies on this).

- Andreas