On Mon, 20 Oct 2014, Vittorio Giovara wrote:
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?
I don't think an additional check would be useful, the only way this would
fail is if av_packet_get_side_data would return an inconsistent
combination of values. So false positive according to me.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
