Only append SPS/PPS to bitstream if supplied out of bound.

Only append SPS/PPS to bitstream if supplied out of band.

BUG=chromium:721597
Review-Url: https://codereview.webrtc.org/2945853002
Cr-Commit-Position: refs/heads/master@{#18701}
Committed: https://chromium.googlesource.com/external/webrtc/+/83c97da5931956ae8596d2efcfdea282fdc86d2d

[philipel, 2017-06-19 14:50:54]

philipel@webrtc.org changed reviewers:
+ stefan@webrtc.org, terelius@webrtc.org

[philipel, 2017-06-19 14:50:54]

https://codereview.webrtc.org/2945853002/diff/1/webrtc/video/rtp_video_stream...
File webrtc/video/rtp_video_stream_receiver_unittest.cc (left):

https://codereview.webrtc.org/2945853002/diff/1/webrtc/video/rtp_video_stream...
webrtc/video/rtp_video_stream_receiver_unittest.cc:244:
idr_packet.type.Video.is_first_packet_in_frame = true;
Set on line 241

https://codereview.webrtc.org/2945853002/diff/1/webrtc/video/rtp_video_stream...
webrtc/video/rtp_video_stream_receiver_unittest.cc:246:
idr_packet.type.Video.codec = kRtpVideoH264;
Set by GetDefaultPacket()

[stefan-webrtc, 2017-06-20 08:56:31]

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
File webrtc/modules/video_coding/h264_sps_pps_tracker.cc (right):

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
webrtc/modules/video_coding/h264_sps_pps_tracker.cc:82: RTC_DCHECK_LE(0,
pps->second.size);
I'd prefer RTC_DCHECK_GT(sps->second.size, 0)

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
webrtc/modules/video_coding/h264_sps_pps_tracker.cc:86: FALLTHROUGH();
No point in FALLTHROUGH now, right? Might as well break

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
webrtc/modules/video_coding/h264_sps_pps_tracker.cc:88: default: { break; }
Remove {}

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
webrtc/modules/video_coding/h264_sps_pps_tracker.cc:170: packet->height =
sps->second.height;
Should this be moved up lines 125-137? Maybe also with a comment of why we do
it?

[philipel, 2017-06-20 11:27:34]

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
File webrtc/modules/video_coding/h264_sps_pps_tracker.cc (right):

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
webrtc/modules/video_coding/h264_sps_pps_tracker.cc:82: RTC_DCHECK_LE(0,
pps->second.size);
On 2017/06/20 08:56:31, stefan-webrtc wrote:
> I'd prefer RTC_DCHECK_GT(sps->second.size, 0)

Done.

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
webrtc/modules/video_coding/h264_sps_pps_tracker.cc:86: FALLTHROUGH();
On 2017/06/20 08:56:31, stefan-webrtc wrote:
> No point in FALLTHROUGH now, right? Might as well break

Done.

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
webrtc/modules/video_coding/h264_sps_pps_tracker.cc:88: default: { break; }
On 2017/06/20 08:56:31, stefan-webrtc wrote:
> Remove {}

Done.

https://codereview.webrtc.org/2945853002/diff/1/webrtc/modules/video_coding/h...
webrtc/modules/video_coding/h264_sps_pps_tracker.cc:170: packet->height =
sps->second.height;
On 2017/06/20 08:56:31, stefan-webrtc wrote:
> Should this be moved up lines 125-137? Maybe also with a comment of why we do
> it?

Done.

[stefan-webrtc, 2017-06-20 15:09:05]

lgtm

[terelius, 2017-06-20 15:12:54]

"out of bound" -> "out of band" in CL description?

[philipel, 2017-06-20 15:14:18]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2017-06-20 15:14:22]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[philipel, 2017-06-20 15:14:40]

Description was changed from

==========
Only append SPS/PPS to bitstream if supplied out of bound.

BUG=chromium:721597
==========

to

==========
Only append SPS/PPS to bitstream if supplied out of band.

BUG=chromium:721597
==========

[commit-bot: I haz the power, 2017-06-20 15:43:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-20 15:43:56]

Try jobs failed on following builders:
  linux_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/25736)

[philipel, 2017-06-21 12:00:30]

The CQ bit was checked by philipel@webrtc.org

[philipel, 2017-06-21 12:00:30]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2945853002/#ps60001 (title:
"delete[] sps/pps data in unittests")

[commit-bot: I haz the power, 2017-06-21 12:00:41]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-06-21 12:21:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-21 12:21:59]

Try jobs failed on following builders:
  android_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/25158)

[philipel, 2017-06-21 14:20:17]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2017-06-21 14:20:31]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-06-21 14:22:40]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498054817045410,
"parent_rev": "548813a4a76d448d408919e38449266957fa3b8f", "commit_rev":
"83c97da5931956ae8596d2efcfdea282fdc86d2d"}

[commit-bot: I haz the power, 2017-06-21 14:22:43]

Description was changed from

==========
Only append SPS/PPS to bitstream if supplied out of band.

BUG=chromium:721597
==========

to

==========
Only append SPS/PPS to bitstream if supplied out of band.

BUG=chromium:721597

Review-Url: https://codereview.webrtc.org/2945853002
Cr-Commit-Position: refs/heads/master@{#18701}
Committed:
https://chromium.googlesource.com/external/webrtc/+/83c97da5931956ae8596d2efc...
==========

[commit-bot: I haz the power, 2017-06-21 14:22:44]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/83c97da5931956ae8596d2efc...