On Sun, Oct 19, 2014 at 7:18 PM, Martin Storsjö <[email protected]> wrote: > On Sat, 18 Oct 2014, Vittorio Giovara wrote: > >> CC: [email protected] >> Bug-Id: CID 1035715 >> --- >> libavformat/rtpenc.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/libavformat/rtpenc.c b/libavformat/rtpenc.c >> index 647a807..63b76f9 100644 >> --- a/libavformat/rtpenc.c >> +++ b/libavformat/rtpenc.c >> @@ -569,8 +569,13 @@ static int rtp_write_packet(AVFormatContext *s1, >> AVPacket *pkt) >> const uint8_t *mb_info = >> av_packet_get_side_data(pkt, AV_PKT_DATA_H263_MB_INFO, >> &mb_info_size); >> - ff_rtp_send_h263_rfc2190(s1, pkt->data, size, mb_info, >> mb_info_size); >> - break; >> + if (!mb_info) { >> + av_log(s1, AV_LOG_WARNING, "No MB info found.\n") >> + } else { >> + ff_rtp_send_h263_rfc2190(s1, pkt->data, size, >> + mb_info, mb_info_size); >> + break; >> + } >> } >> /* Fallthrough */ >> case AV_CODEC_ID_H263P: >> -- >> 1.9.3 (Apple Git-50) > > > No, this is wrong. The packetizer might succeed just fine without the MB > info, if the input packet happens to have resync markers often enough (e.g. > for small resolutions/bitrates etc). (One can't always generate them often > enough, so that's why the MB info is needed for it to be reliable, but in > case the input is packetizable we shouldn't stop here.)
I see, thanks for the explanation. > Have a look at rtpenc_h263_rfc2190.c - we check mb_info_size before using > it. The assumption that coverity probably is missing is that if mb_info is > NULL, then mb_info_size is assumed to be 0 to make sure we don't actually > read from it. Ideally coverity would need to know this. I didn't check what > coverity actually is complaining about here, but one way of silencing it > would be to add a check for mb_info being non-null within > rtpenc_h263_rfc2190 before actually using it (next to the mb_info_count > checks, which clearly shows why it's a false positive). Yes you're correct, coverity does not notice that mb_info is not used when mb_info_size is 0, probably because of the indirection in mb_info_count. Do you suggest to mark is a false positive or the additional check might be actually useful under certain conditions? -- Vittorio _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
