Reland of quest keyframes more frequently on stream start/decoding error.

Reland of quest keyframes more frequently on stream start/decoding error. (patchset #1 id:1 of https://codereview.chromium.org/2994043002/ )

Reason for revert:
Create fix CL.

Original issue's description:
> Revert of Request keyframes more frequently on stream start/decoding error. (patchset #1 id:1 of https://codereview.webrtc.org/2993793002/ )
> 
> Reason for revert:
> Broke downstream test that was waiting for 5 keyframes to be received within 10 seconds. Maybe the issue is that "stats_callback_->OnCompleteFrame(frame->num_references == 0, ..." was changed to "frame->is_keyframe()"?
> 
> Original issue's description:
> > Request keyframes more frequently on stream start/decoding error.
> > 
> > In this CL:
> >  - Added FrameObject::is_keyframe() convinience function.
> >  - Moved logic to request keyframes on decoding error from VideoReceived to
> >    VideoReceiveStream.
> >  - Added keyframe_required as a parameter to FrameBuffer::NextFrame.
> > 
> > BUG=webrtc:8074
> > 
> > Review-Url: https://codereview.webrtc.org/2993793002
> > Cr-Commit-Position: refs/heads/master@{#19280}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/26b48043581735eed6e36b95fae6f5b1bcf8cfb5
> 
> TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,philipel@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:8074
> 
> Review-Url: https://codereview.webrtc.org/2994043002
> Cr-Commit-Position: refs/heads/master@{#19295}
> Committed: https://chromium.googlesource.com/external/webrtc/+/77a983185f57628cd5955bd2c0a1bf71c30439bb

TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=webrtc:8074
Review-Url: https://codereview.webrtc.org/2996823002
Cr-Commit-Position: refs/heads/master@{#19324}
Committed: https://chromium.googlesource.com/external/webrtc/+/628ac5964e32e66083a6ab14dceac6cb2cabe345

[philipel, 2017-08-10 08:36:35]

Created Reland of quest keyframes more frequently on stream start/decoding
error.

[philipel, 2017-08-11 08:41:28]

Stefan, PTAL

[philipel, 2017-08-11 08:49:22]

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

[commit-bot: I haz the power, 2017-08-11 08:49:32]

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, 2017-08-11 08:49:33]

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

[commit-bot: I haz the power, 2017-08-11 08:49:34]

Dry run: This issue passed the CQ dry run.

[philipel, 2017-08-11 08:50:35]

Description was changed from

==========
Reland of quest keyframes more frequently on stream start/decoding error.
(patchset #1 id:1 of https://codereview.chromium.org/2994043002/ )

Reason for revert:
Create fix CL.

Original issue's description:
> Revert of Request keyframes more frequently on stream start/decoding error.
(patchset #1 id:1 of https://codereview.webrtc.org/2993793002/ )
> 
> Reason for revert:
> Broke downstream test that was waiting for 5 keyframes to be received within
10 seconds. Maybe the issue is that
"stats_callback_->OnCompleteFrame(frame->num_references == 0, ..." was changed
to "frame->is_keyframe()"?
> 
> Original issue's description:
> > Request keyframes more frequently on stream start/decoding error.
> > 
> > In this CL:
> >  - Added FrameObject::is_keyframe() convinience function.
> >  - Moved logic to request keyframes on decoding error from VideoReceived to
> >    VideoReceiveStream.
> >  - Added keyframe_required as a parameter to FrameBuffer::NextFrame.
> > 
> > BUG=webrtc:8074
> > 
> > Review-Url: https://codereview.webrtc.org/2993793002
> > Cr-Commit-Position: refs/heads/master@{#19280}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/26b48043581735eed6e36b95f...
> 
>
TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,philipel@webrt...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:8074
> 
> Review-Url: https://codereview.webrtc.org/2994043002
> Cr-Commit-Position: refs/heads/master@{#19295}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/77a983185f57628cd5955bd2c...

TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,deadbeef@webrt...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:8074
==========

to

==========
Reland of quest keyframes more frequently on stream start/decoding error.
(patchset #1 id:1 of https://codereview.chromium.org/2994043002/ )

Reason for revert:
Create fix CL.

Original issue's description:
> Revert of Request keyframes more frequently on stream start/decoding error.
(patchset #1 id:1 of https://codereview.webrtc.org/2993793002/ )
> 
> Reason for revert:
> Broke downstream test that was waiting for 5 keyframes to be received within
10 seconds. Maybe the issue is that
"stats_callback_->OnCompleteFrame(frame->num_references == 0, ..." was changed
to "frame->is_keyframe()"?
> 
> Original issue's description:
> > Request keyframes more frequently on stream start/decoding error.
> > 
> > In this CL:
> >  - Added FrameObject::is_keyframe() convinience function.
> >  - Moved logic to request keyframes on decoding error from VideoReceived to
> >    VideoReceiveStream.
> >  - Added keyframe_required as a parameter to FrameBuffer::NextFrame.
> > 
> > BUG=webrtc:8074
> > 
> > Review-Url: https://codereview.webrtc.org/2993793002
> > Cr-Commit-Position: refs/heads/master@{#19280}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/26b48043581735eed6e36b95f...
> 
>
TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,philipel@webrt...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:8074
> 
> Review-Url: https://codereview.webrtc.org/2994043002
> Cr-Commit-Position: refs/heads/master@{#19295}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/77a983185f57628cd5955bd2c...

TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,deadbeef@webrt...
# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=webrtc:8074
==========

[philipel, 2017-08-11 08:50:43]

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

[commit-bot: I haz the power, 2017-08-11 08:50:51]

Dry run: CQ is trying da patch.

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

[stefan-webrtc, 2017-08-11 09:28:00]

Remember to file a bug about fixing this downstream. :(

lgtm

https://codereview.webrtc.org/2996823002/diff/170001/webrtc/video/video_recei...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/2996823002/diff/170001/webrtc/video/video_recei...
webrtc/video/video_receive_stream.cc:513: } else if (keyframe_required_ ==
false) {
!keyframe_required_

[commit-bot: I haz the power, 2017-08-11 09:28:42]

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

[commit-bot: I haz the power, 2017-08-11 09:28:43]

Dry run: This issue passed the CQ dry run.

[philipel, 2017-08-11 10:38:35]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2017-08-11 10:38:42]

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-08-11 10:41:45]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1502447915238220,
"parent_rev": "a1ebeb5b7a55dce07307139eaafe158541a0a8a9", "commit_rev":
"628ac5964e32e66083a6ab14dceac6cb2cabe345"}

[commit-bot: I haz the power, 2017-08-11 10:41:50]

Description was changed from

==========
Reland of quest keyframes more frequently on stream start/decoding error.
(patchset #1 id:1 of https://codereview.chromium.org/2994043002/ )

Reason for revert:
Create fix CL.

Original issue's description:
> Revert of Request keyframes more frequently on stream start/decoding error.
(patchset #1 id:1 of https://codereview.webrtc.org/2993793002/ )
> 
> Reason for revert:
> Broke downstream test that was waiting for 5 keyframes to be received within
10 seconds. Maybe the issue is that
"stats_callback_->OnCompleteFrame(frame->num_references == 0, ..." was changed
to "frame->is_keyframe()"?
> 
> Original issue's description:
> > Request keyframes more frequently on stream start/decoding error.
> > 
> > In this CL:
> >  - Added FrameObject::is_keyframe() convinience function.
> >  - Moved logic to request keyframes on decoding error from VideoReceived to
> >    VideoReceiveStream.
> >  - Added keyframe_required as a parameter to FrameBuffer::NextFrame.
> > 
> > BUG=webrtc:8074
> > 
> > Review-Url: https://codereview.webrtc.org/2993793002
> > Cr-Commit-Position: refs/heads/master@{#19280}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/26b48043581735eed6e36b95f...
> 
>
TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,philipel@webrt...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:8074
> 
> Review-Url: https://codereview.webrtc.org/2994043002
> Cr-Commit-Position: refs/heads/master@{#19295}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/77a983185f57628cd5955bd2c...

TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,deadbeef@webrt...
# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=webrtc:8074
==========

to

==========
Reland of quest keyframes more frequently on stream start/decoding error.
(patchset #1 id:1 of https://codereview.chromium.org/2994043002/ )

Reason for revert:
Create fix CL.

Original issue's description:
> Revert of Request keyframes more frequently on stream start/decoding error.
(patchset #1 id:1 of https://codereview.webrtc.org/2993793002/ )
> 
> Reason for revert:
> Broke downstream test that was waiting for 5 keyframes to be received within
10 seconds. Maybe the issue is that
"stats_callback_->OnCompleteFrame(frame->num_references == 0, ..." was changed
to "frame->is_keyframe()"?
> 
> Original issue's description:
> > Request keyframes more frequently on stream start/decoding error.
> > 
> > In this CL:
> >  - Added FrameObject::is_keyframe() convinience function.
> >  - Moved logic to request keyframes on decoding error from VideoReceived to
> >    VideoReceiveStream.
> >  - Added keyframe_required as a parameter to FrameBuffer::NextFrame.
> > 
> > BUG=webrtc:8074
> > 
> > Review-Url: https://codereview.webrtc.org/2993793002
> > Cr-Commit-Position: refs/heads/master@{#19280}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/26b48043581735eed6e36b95f...
> 
>
TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,philipel@webrt...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:8074
> 
> Review-Url: https://codereview.webrtc.org/2994043002
> Cr-Commit-Position: refs/heads/master@{#19295}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/77a983185f57628cd5955bd2c...

TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,deadbeef@webrt...
# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=webrtc:8074

Review-Url: https://codereview.webrtc.org/2996823002
Cr-Commit-Position: refs/heads/master@{#19324}
Committed:
https://chromium.googlesource.com/external/webrtc/+/628ac5964e32e66083a6ab14d...
==========

[commit-bot: I haz the power, 2017-08-11 10:41:51]

Committed patchset #2 (id:170001) as
https://chromium.googlesource.com/external/webrtc/+/628ac5964e32e66083a6ab14d...

[tkchin_webrtc, 2017-08-17 17:12:37]

tkchin@webrtc.org changed reviewers:
+ tkchin@webrtc.org

[tkchin_webrtc, 2017-08-17 17:12:38]

https://codereview.webrtc.org/2996823002/diff/170001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_receiver.cc (left):

https://codereview.webrtc.org/2996823002/diff/170001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_receiver.cc:326: // Check for failed decoding,
run frame type request callback if needed.
Removing the check for failed decoding causes an issue with the iOS h264 decoder
when it comes back alive from the background where it takes a long time to
receive a keyframe.

[tkchin_webrtc, 2017-08-17 17:14:35]

A revert of this CL (patchset #2 id:170001) has been created in
https://codereview.webrtc.org/2995153002/ by tkchin@webrtc.org.

The reason for reverting is: Causes iOS H264 calls received in the background to
have increased delay before being able to decode stream from sender due to not
having a keyframe..

[philipel, 2017-08-18 08:38:20]

https://codereview.webrtc.org/2996823002/diff/170001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_receiver.cc (left):

https://codereview.webrtc.org/2996823002/diff/170001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_receiver.cc:326: // Check for failed decoding,
run frame type request callback if needed.
On 2017/08/17 17:12:38, tkchin_webrtc wrote:
> Removing the check for failed decoding causes an issue with the iOS h264
decoder
> when it comes back alive from the background where it takes a long time to
> receive a keyframe.

This piece of logic was moved to the VideoReceviveStream::Decode function, it
should work just the same.