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

Issue 2458673002: Configure OpenH264 sSliceArgument.uiSliceNum set to 1. (Closed)

Created:
4 years, 1 month ago by sprang_webrtc
Modified:
4 years, 1 month ago
Reviewers:
hbos
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, zhengzhonghou_agora.io, video-team_agora.io, stefan-webrtc, mflodman
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

It seems that if encoder_params.sSpatialLayers[0].sSliceArgument.uiSliceNum is configured to number of cores as determined by openh264 (or any number > 1 in my local tests), frame rate statistics will be mucked up (apparently thousands of frames per second) and quality will bottom out because bits per frame is then very low. BUG=webrtc:6583 Committed: https://crrev.com/ca27f9d5b9d0121d1a38417268fc2316dcd30d49 Cr-Commit-Position: refs/heads/master@{#14842}

Patch Set 1 #

Total comments: 3

Patch Set 2 : Set uiSliceNum = 1 until rate controller is fixed #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -1 line) Patch
M webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc View 1 2 chunks +5 lines, -1 line 0 comments Download

Messages

Total messages: 13 (6 generated)
sprang_webrtc
4 years, 1 month ago (2016-10-27 12:32:09 UTC) #3
hbos
LGTM Remember to do the same change in Chromium so that the other encoder does ...
4 years, 1 month ago (2016-10-28 09:01:03 UTC) #4
sprang_webrtc
https://codereview.webrtc.org/2458673002/diff/1/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc File webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc (right): https://codereview.webrtc.org/2458673002/diff/1/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc#newcode447 webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc:447: encoder_params.iMultipleThreadIdc; On 2016/10/28 09:01:03, hbos wrote: > Do we ...
4 years, 1 month ago (2016-10-31 09:22:29 UTC) #5
hbos
https://codereview.webrtc.org/2458673002/diff/1/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc File webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc (right): https://codereview.webrtc.org/2458673002/diff/1/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc#newcode447 webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc:447: encoder_params.iMultipleThreadIdc; On 2016/10/31 09:22:29, språng wrote: > On 2016/10/28 ...
4 years, 1 month ago (2016-10-31 09:23:34 UTC) #6
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/2458673002/20001
4 years, 1 month ago (2016-10-31 09:40:20 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 1 month ago (2016-10-31 10:43:41 UTC) #11
commit-bot: I haz the power
4 years, 1 month ago (2016-10-31 10:43:53 UTC) #13
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/ca27f9d5b9d0121d1a38417268fc2316dcd30d49
Cr-Commit-Position: refs/heads/master@{#14842}

Powered by Google App Engine
This is Rietveld 408576698