Use sps and pps to determine decodability of H.264 frames.

Use sps and pps to determine decodability of H.264 frames.

NOPRESUBMIT=true
BUG=webrtc:6208
R=philipel@webrtc.org

Committed: https://crrev.com/17b02633666f2f5d7e78767ad5674c728d639c26
Cr-Commit-Position: refs/heads/master@{#14458}

[stefan-webrtc, 2016-09-14 09:58:59]

.

[stefan-webrtc, 2016-09-14 09:59:54]

The CQ bit was checked by stefan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 10:00:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[stefan-webrtc, 2016-09-14 10:03:17]

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

[commit-bot: I haz the power, 2016-09-14 10:07:32]

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

[commit-bot: I haz the power, 2016-09-14 10:07:33]

Dry run: Try jobs failed on following builders:
  linux_ubsan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/5027)
  mac_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/18223)

[stefan-webrtc, 2016-09-14 14:59:52]

.

[stefan-webrtc, 2016-09-14 15:01:03]

The CQ bit was checked by stefan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 15:01:11]

Dry run: 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, 2016-09-14 15:11:11]

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

[commit-bot: I haz the power, 2016-09-14 15:11:12]

Dry run: Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/8355)

[stefan-webrtc, 2016-09-15 11:27:56]

ping :)

[philipel, 2016-09-15 12:52:46]

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/decoding_state.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:82: for (const NaluInfo& nalu :
nalus) {
"const NaluInfo& nalu : frame->GetNaluInfos()" to avoid unnecessary copy.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:82: for (const NaluInfo& nalu :
nalus) {
Maybe clean up old sps/pps? How many sps/pps can we expect the decoder to keep
track of?

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:314: std::set<int> new_sps;
Remove |new_sps| and insert new sps into |received_sps_| instead.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:315: std::map<int, int> new_pps;
Remove |new_pps| and insert new pps into |received_pps_| instead.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/decoding_state.h (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.h:85: std::map<int, int>
received_pps_;
Can sps/pps id wrap?

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/frame_buffer.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/frame_buffer.cc:15: #include <vector>
Included in the .h file.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/jitter_buffer_unittest.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/jitter_buffer_unittest.cc:1182: // Not decodable
since pps is missing.
since pps and sps is missing

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/session_info.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/session_info.cc:13: #include <vector>
in .h file

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/session_info.h (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/session_info.h:92: std::vector<NaluInfo>
GetNaluInfos() const;
Return "rtc::ArrayView<NaluInfo>" instead.

[stefan-webrtc, 2016-09-30 09:36:02]

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/decoding_state.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:82: for (const NaluInfo& nalu :
nalus) {
On 2016/09/15 12:52:46, philipel wrote:
> Maybe clean up old sps/pps? How many sps/pps can we expect the decoder to keep
> track of?

It must be able to track all of them according to the standard.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:82: for (const NaluInfo& nalu :
nalus) {
On 2016/09/15 12:52:46, philipel wrote:
> "const NaluInfo& nalu : frame->GetNaluInfos()" to avoid unnecessary copy. 

Done.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:314: std::set<int> new_sps;
On 2016/09/15 12:52:46, philipel wrote:
> Remove |new_sps| and insert new sps into |received_sps_| instead.

We can't insert them now since the frame hasn't been decoded yet, so we can't
considered these sps/pps received until then. We should however consider them
for this frame as they're part of it.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/decoding_state.h (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.h:85: std::map<int, int>
received_pps_;
On 2016/09/15 12:52:46, philipel wrote:
> Can sps/pps id wrap?

Yes, but when they do they replace the old ones.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/frame_buffer.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/frame_buffer.cc:15: #include <vector>
On 2016/09/15 12:52:46, philipel wrote:
> Included in the .h file.

Done.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/jitter_buffer_unittest.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/jitter_buffer_unittest.cc:1182: // Not decodable
since pps is missing.
On 2016/09/15 12:52:46, philipel wrote:
> since pps and sps is missing

Done.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/session_info.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/session_info.cc:13: #include <vector>
On 2016/09/15 12:52:46, philipel wrote:
> in .h file

Done.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/session_info.h (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/session_info.h:92: std::vector<NaluInfo>
GetNaluInfos() const;
On 2016/09/15 12:52:46, philipel wrote:
> Return "rtc::ArrayView<NaluInfo>" instead.

I don't think it's possible since a session info can contain several packets
each having an array of nalu infos.

[philipel, 2016-09-30 10:52:52]

lgtm

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/decoding_state.cc (right):

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:82: for (const NaluInfo& nalu :
nalus) {
On 2016/09/30 09:36:02, stefan-webrtc (holmer) wrote:
> On 2016/09/15 12:52:46, philipel wrote:
> > Maybe clean up old sps/pps? How many sps/pps can we expect the decoder to
keep
> > track of?
> 
> It must be able to track all of them according to the standard.

Acknowledged.

https://codereview.webrtc.org/2341713002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/decoding_state.cc:314: std::set<int> new_sps;
On 2016/09/30 09:36:02, stefan-webrtc (holmer) wrote:
> On 2016/09/15 12:52:46, philipel wrote:
> > Remove |new_sps| and insert new sps into |received_sps_| instead.
> 
> We can't insert them now since the frame hasn't been decoded yet, so we can't
> considered these sps/pps received until then. We should however consider them
> for this frame as they're part of it.

Acknowledged.

[stefan-webrtc, 2016-09-30 11:10:44]

The CQ bit was checked by stefan@webrtc.org

[commit-bot: I haz the power, 2016-09-30 11:10:48]

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, 2016-09-30 11:34:01]

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

[commit-bot: I haz the power, 2016-09-30 11:34:02]

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

[stefan-webrtc, 2016-09-30 11:40:42]

Description was changed from

==========
Use sps and pps to determine decodability of H.264 frames.

BUG=webrtc:6208
==========

to

==========
Use sps and pps to determine decodability of H.264 frames.

NOPRESUBMIT=true
BUG=webrtc:6208
==========

[stefan-webrtc, 2016-09-30 11:47:11]

The CQ bit was checked by stefan@webrtc.org

[commit-bot: I haz the power, 2016-09-30 11:47:22]

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, 2016-09-30 13:11:23]

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

[commit-bot: I haz the power, 2016-09-30 13:11:24]

Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[stefan-webrtc, 2016-09-30 13:24:30]

Description was changed from

==========
Use sps and pps to determine decodability of H.264 frames.

NOPRESUBMIT=true
BUG=webrtc:6208
==========

to

==========
Use sps and pps to determine decodability of H.264 frames.

NOPRESUBMIT=true
BUG=webrtc:6208
R=philipel@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/17b02633666f2f5d7e78767ad...
==========

[stefan-webrtc, 2016-09-30 13:24:31]

Committed patchset #4 (id:60001) manually as
17b02633666f2f5d7e78767ad5674c728d639c26 (presubmit successful).

[commit-bot: I haz the power, 2016-09-30 13:24:32]

Description was changed from

==========
Use sps and pps to determine decodability of H.264 frames.

NOPRESUBMIT=true
BUG=webrtc:6208
R=philipel@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/17b02633666f2f5d7e78767ad...
==========

to

==========
Use sps and pps to determine decodability of H.264 frames.

NOPRESUBMIT=true
BUG=webrtc:6208
R=philipel@webrtc.org

Committed: https://crrev.com/17b02633666f2f5d7e78767ad5674c728d639c26
Cr-Commit-Position: refs/heads/master@{#14458}
==========

[stefan-webrtc, 2016-09-30 16:06:22]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.webrtc.org/2381233004/ by stefan@webrtc.org.

The reason for reverting is: Broke browser_tests, e.g.,
WebRtcVideoQualityBrowserTests/WebRtcVideoQualityBrowserTest.MANUAL_TestVideoQualityH264.