Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(316)

Issue 2996153003: Reland of quest keyframes more frequently on stream start/decoding error. (Closed)

Created:
3 years, 4 months ago by philipel
Modified:
3 years, 4 months ago
CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, tterriberry_mozilla.com, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Reland of quest keyframes more frequently on stream start/decoding error. (patchset #1 id:1 of https://codereview.chromium.org/2995153002/ ) Reason for revert: iOS workaround. Original issue's description: > Revert of quest keyframes more frequently on stream start/decoding error. (patchset #2 id:170001 of https://codereview.webrtc.org/2996823002/ ) > > Reason for revert: > 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. > > Original issue's description: > > 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 > > TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,deadbeef@webrtc.org,philipel@webrtc.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG=webrtc:8074 > > Review-Url: https://codereview.webrtc.org/2995153002 > Cr-Commit-Position: refs/heads/master@{#19392} > Committed: https://chromium.googlesource.com/external/webrtc/+/53959fcc2ba580e7c87231708e5b4af7906f6836 TBR=terelius@webrtc.org,stefan@webrtc.org,noahric@chromium.org,deadbeef@webrtc.org,tkchin@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. BUG=webrtc:8074 Review-Url: https://codereview.webrtc.org/2996153003 Cr-Commit-Position: refs/heads/master@{#19410} Committed: https://chromium.googlesource.com/external/webrtc/+/3042c2d5e0e400d96437e041151995fd96f46ff4

Patch Set 1 #

Patch Set 2 : Workaround for H264 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -31 lines) Patch
M webrtc/modules/video_coding/frame_buffer2.h View 1 1 chunk +2 lines, -1 line 0 comments Download
M webrtc/modules/video_coding/frame_buffer2.cc View 1 4 chunks +8 lines, -3 lines 0 comments Download
M webrtc/modules/video_coding/frame_buffer2_unittest.cc View 1 2 chunks +16 lines, -2 lines 0 comments Download
M webrtc/modules/video_coding/frame_object.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/modules/video_coding/video_receiver.cc View 1 1 chunk +1 line, -18 lines 0 comments Download
M webrtc/video/video_receive_stream.h View 1 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/video/video_receive_stream.cc View 1 3 chunks +19 lines, -7 lines 0 comments Download

Messages

Total messages: 14 (8 generated)
philipel
Created Reland of quest keyframes more frequently on stream start/decoding error.
3 years, 4 months ago (2017-08-18 10:49:04 UTC) #1
philipel
Stefan, PTAL
3 years, 4 months ago (2017-08-18 11:19:58 UTC) #3
stefan-webrtc
I'm not sure what is iOS specific about this, but the fix lgtm.
3 years, 4 months ago (2017-08-18 11:37:15 UTC) #6
philipel
On 2017/08/18 11:37:15, stefan-webrtc wrote: > I'm not sure what is iOS specific about this, ...
3 years, 4 months ago (2017-08-18 11:39:37 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2996153003/210001
3 years, 4 months ago (2017-08-18 11:52:40 UTC) #11
commit-bot: I haz the power
3 years, 4 months ago (2017-08-18 11:55:11 UTC) #14
Message was sent while issue was closed.
Committed patchset #2 (id:210001) as
https://chromium.googlesource.com/external/webrtc/+/3042c2d5e0e400d96437e0411...

Powered by Google App Engine
This is Rietveld 408576698