Stashed frames are now retried in a loop rather than recursively.

Stashed frames are now retried in a loop rather than recursively.

BUG=none

Review-Url: https://codereview.webrtc.org/2841913002
Cr-Commit-Position: refs/heads/master@{#17890}
Committed: https://chromium.googlesource.com/external/webrtc/+/afcf7f5591de47063f7da3507ea83b48970a6e1f

[philipel, 2017-04-25 15:16:59]

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

[stefan-webrtc, 2017-04-25 15:27:49]

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

https://codereview.webrtc.org/2841913002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/rtp_frame_reference_finder.cc:43:
stashed_frames_.pop_back();
Here we pop_back(), before we did pop_front()?

https://codereview.webrtc.org/2841913002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/rtp_frame_reference_finder.cc:580: void
RtpFrameReferenceFinder::CompletedFrameVp9(RtpFrameObject* frame) {
CompletedFrameVp9 and CompletedFrameVp8 don't really say what these methods do.
I think we can pick a better name for them...

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

https://codereview.webrtc.org/2841913002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/rtp_frame_reference_finder.h:68: enum FrameDecision
{ kStash, kSend, kDrop };
kSend sounds like it means that we will send it over the network. Is a better
name kDecode?

[philipel, 2017-04-26 12:19:17]

Patchset #4 (id:60001) has been deleted

[philipel, 2017-04-26 12:22:27]

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

https://codereview.webrtc.org/2841913002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/rtp_frame_reference_finder.cc:43:
stashed_frames_.pop_back();
On 2017/04/25 15:27:48, stefan-webrtc wrote:
> Here we pop_back(), before we did pop_front()?

The order has changed so new frames are at the front and old frames are at the
back. It makes it easier to iterate over the frames in the order we want since
we don't have to use rbegin() and rend().

https://codereview.webrtc.org/2841913002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/rtp_frame_reference_finder.cc:580: void
RtpFrameReferenceFinder::CompletedFrameVp9(RtpFrameObject* frame) {
On 2017/04/25 15:27:48, stefan-webrtc wrote:
> CompletedFrameVp9 and CompletedFrameVp8 don't really say what these methods
do.
> I think we can pick a better name for them...

Renamed CompletedFrameVp9 to UnwrapPictureIds since that is all this function
does now.

Renamed CompletedFrameVp8 to UpdateLayerInfoVp8, which I think is a fairly
accurate/descriptive name.

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

https://codereview.webrtc.org/2841913002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/rtp_frame_reference_finder.h:68: enum FrameDecision
{ kStash, kSend, kDrop };
On 2017/04/25 15:27:48, stefan-webrtc wrote:
> kSend sounds like it means that we will send it over the network. Is a better
> name kDecode?

Agree, change to kHandOff since we don't actually send it to the decoder, only
to whoever is interested in the complete frames (in our case the frame buffer).

https://codereview.webrtc.org/2841913002/diff/80001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/rtp_frame_reference_finder.cc (right):

https://codereview.webrtc.org/2841913002/diff/80001/webrtc/modules/video_codi...
webrtc/modules/video_coding/rtp_frame_reference_finder.cc:42: FrameDecision
decision = ManageFrameInternal(frame.get());
I check if the incoming frame will be handed off via the callback, and only then
do we retry stashed frames. Otherwise we will always retry all stashed frames
even if the incoming frame didn't result in a hand off, which is just a waste of
CPU.

[stefan-webrtc, 2017-04-26 13:24:08]

lgtm

[philipel, 2017-04-26 13:25:49]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2017-04-26 13:26:00]

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-04-26 13:40:14]

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

[commit-bot: I haz the power, 2017-04-26 13:40:15]

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

[philipel, 2017-04-26 14:16:18]

The CQ bit was checked by philipel@webrtc.org

[philipel, 2017-04-26 14:16:18]

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/2841913002/#ps120001 (title:
"Apparently RTC_NOTREACHED is not enough for the win compiler.")

[commit-bot: I haz the power, 2017-04-26 14:16:25]

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-04-26 15:17:36]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493216178425640,
"parent_rev": "114c1b3afaced323fdb0a15ce026d9a8d50bb230", "commit_rev":
"afcf7f5591de47063f7da3507ea83b48970a6e1f"}

[commit-bot: I haz the power, 2017-04-26 15:17:39]

Description was changed from

==========
Stashed frames are now retried in a loop rather than recursively.

BUG=none
==========

to

==========
Stashed frames are now retried in a loop rather than recursively.

BUG=none

Review-Url: https://codereview.webrtc.org/2841913002
Cr-Commit-Position: refs/heads/master@{#17890}
Committed:
https://chromium.googlesource.com/external/webrtc/+/afcf7f5591de47063f7da3507...
==========

[commit-bot: I haz the power, 2017-04-26 15:17:41]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/external/webrtc/+/afcf7f5591de47063f7da3507...