Sanity check parsed QP values from H264 bitstream

webrtc/common_video/h264/h264_bitstream_parser.cc

Index: webrtc/common_video/h264/h264_bitstream_parser.cc
diff --git a/webrtc/common_video/h264/h264_bitstream_parser.cc b/webrtc/common_video/h264/h264_bitstream_parser.cc
index 048cdd2f6ea58da64b7d4b37507c2e6078a88865..ab97a9081975cde72f041107cdcf937cb94dee65 100644
--- a/webrtc/common_video/h264/h264_bitstream_parser.cc
+++ b/webrtc/common_video/h264/h264_bitstream_parser.cc
@@ -251,6 +251,12 @@ H264BitstreamParser::Result H264BitstreamParser::ParseNonParameterSetNalu(
   int32_t last_slice_qp_delta;
   RETURN_INV_ON_FAIL(
       slice_reader.ReadSignedExponentialGolomb(&last_slice_qp_delta));
+  if (last_slice_qp_delta > 52 || last_slice_qp_delta < -52) {

[magjed_webrtc, 2016/11/28 16:41:32]

Is the constant 52 specified by the spec? We should name it something, like
kMaxAbsQpDeltaValue.

[pbos-webrtc, 2016/11/28 17:15:45]

I think you should also add range checking in GetLastSliceQp which should be the
last step, e.g. something like:

26 + pps_->pic_init_qp_minus26 + *last_slice_qp_delta_ in [0,51]? Otherwise
return false.

If the pps_ should have been set before, then you can make this range check
here.

[kthelgason, 2016/11/29 10:27:12]

Yes, and done.

[kthelgason, 2016/11/29 10:27:12]

Yes, and done.
I added range checks in the pps parser instead, seemed more appropriate.

+    // Something has gone wrong, and the parsed value is invalid.
+    LOG(LS_WARNING) << "Parsed QP value out of range.";
+    return kInvalidStream;
+  }
+
   last_slice_qp_delta_ = rtc::Optional<int32_t>(last_slice_qp_delta);
   return kOk;
 }