[FFmpeg-devel] [PATCH 1/2] avformat/utils: Assert that stream_index is valid

[Gyan]

On 24-09-2019 11:38 PM, Marton Balint wrote:
>
>
> On Tue, 24 Sep 2019, Gyan wrote:
>
>>
>>
>> On 24-09-2019 10:01 PM, Andreas Rheinhardt wrote:
>>> There is currently an ordinary check for this (which would lead to a
>>> memleak), but given that no demuxer should ever return a packet with an
>>> invalid stream_index it is more appropriate for this to be an assert.
>>>
>>> FATE passes with this change.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>> ---
>>>   libavformat/utils.c | 6 ++----
>>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>>> index 3983a3f4ce..d8ef5fe54c 100644
>>> --- a/libavformat/utils.c
>>> +++ b/libavformat/utils.c
>>> @@ -884,10 +884,8 @@ int ff_read_packet(AVFormatContext *s, AVPacket 
>>> *pkt)
>>>               continue;
>>>           }
>>>
>>> -        if (pkt->stream_index >= (unsigned)s->nb_streams) {
>>> -            av_log(s, AV_LOG_ERROR, "Invalid stream index %d\n", 
>> pkt->stream_index);
>>> -            continue;
>>> -        }
>>> +        av_assert0(pkt->stream_index < (unsigned)s->nb_streams &&
>>> +                   "Invalid stream index.\n");
>>
>> The mpegts demuxer can send packets from new streams which appear 
>> mid-way. See the check in ffmpeg.c process_input() which discards 
>> these packets.
>
> That checks InputFile->nb_streams not AVFormatContext->nb_streams as 
> far as I see.
>
>> Whereas the patch will lead to an abort in fftools and other user 
>> apps transcoding a live feed.
>
> Are you sure? I think the demuxer always creates a new stream before 
> returning a packet, so AVFormatContext->nb_streams should always be 
> up-to-date when this code runs.

No. I'll have to craft an input and check.

Gyan