[FFmpeg-devel] [PATCH] avformat/mux: Check pkt->stream_index before using it

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Sun May 10 22:35:54 EEST 2020


Anton Khirnov:
> Quoting Marton Balint (2020-05-10 19:45:04)
>>
>>
>> On Sun, 10 May 2020, Anton Khirnov wrote:
>>
>>> Quoting Andreas Rheinhardt (2020-05-08 00:55:00)
>>>> This commit fixes two recent regressions both of which are about using
>>>> pkt->stream_index as index in an AVFormatContext's streams array before
>>>> actually comparing the value with the count of streams in said array.
>>>> 96e5e6abb9851d7a26ba21703955d5826ac857c0 did this in
>>>> prepare_input_packet() and 64063512227c4c87a7d16a1076481dc6baf19841 did
>>>> likewise in write_packets_common().
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>>> ---
>>>> The same error in the same file applied on the same day by two different
>>>> people. How unlikely.
>>>
>>> How is it a regression? Isn't it rather invalid API use?
>>
>> Fun fact: 7b03b65bf0d02519c86750d2da33f413e11cf0c6
>>
>> Yes, it is kind of invalid API use, but since the check is already there, 
>> we should make it actually worthwile.
> 
> lol
> 
> I agree that checking for it is a good idea, obviously, but I wouldn't
> call it a regression.
> 
How about rephrasing the first sentence to: "This commit stops using
pkt->stream_index as index in an AVFormatContext's streams array before
actually comparing the value with the count of streams in said array."
>>
>>>
>>> Not that I object to having a check. But then why is check_packet()
>>> called so deep and not immediately on entry to the muxer?
>>
>> I guess it is not that deep, but recent factorization efforts hidden it a 
>> bit.
> 
> You can see in my original commit it is the very first thing done after
> entering the muxer. Right now it's several function calls deep.
> 
I could make it the very first thing called in write_packets_common().

- Andreas


More information about the ffmpeg-devel mailing list