Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(12)

Issue 3011373002: Updating OpenH264 to v1.7.0 (Closed)

Created:
2 years, 1 month ago by ssilkin
Modified:
2 years ago
CC:
webrtc-reviews_webrtc.org, qiang.lu, niklas.enbom, yujie_mao (webrtc), peah-webrtc
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Updating OpenH264 to v1.7.0 There is bug in AQ in v1.6.0 which causes raising QP to maximum value and results in very poor quality of video no matter of allocated bitrate. To trigger this someone needs to set packetization_mode=0 (or just do not transmit this flag at all) in SDP. In this mode the encoder enables multi-slice and disables AQ partially such that some part of AQ algo still works and leads QP to maximum. The issue is fixed in v1.7.0. BUG=webrtc:8070 Review-Url: https://codereview.webrtc.org/3011373002 Cr-Commit-Position: refs/heads/master@{#20015} Committed: https://webrtc.googlesource.com/src/+/1440c9f70cdd701210652c7758b664ae82a4fef3

Patch Set 1 #

Patch Set 2 : added test for SingleNalUnit mode #

Patch Set 3 : increasing frame size mismatch threshold #

Total comments: 2

Patch Set 4 : moved unit test to separate cl #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M DEPS View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 44 (31 generated)
ssilkin
2 years, 1 month ago (2017-09-20 09:54:25 UTC) #9
brandtr
This lgtm, but try to update the commit message to be on the form: "Short ...
2 years, 1 month ago (2017-09-21 08:17:57 UTC) #10
brandtr
On 2017/09/21 08:17:57, brandtr wrote: > This lgtm, but try to update the commit message ...
2 years, 1 month ago (2017-09-21 08:19:15 UTC) #11
ssilkin
On 2017/09/21 08:19:15, brandtr wrote: > On 2017/09/21 08:17:57, brandtr wrote: > > This lgtm, ...
2 years, 1 month ago (2017-09-21 10:18:26 UTC) #14
hbos
Sorry for the delayed review. Yes we should update chromium to, and we should try ...
2 years ago (2017-09-25 11:02:35 UTC) #28
ssilkin
I moved unit test to separate CL - https://codereview.webrtc.org/3014623002/ - and addressed the notes there.
2 years ago (2017-09-25 15:08:47 UTC) #29
hbos
LGTM, but don't land this without having the chromium bits sorted out
2 years ago (2017-09-25 15:49:34 UTC) #30
hbos
Awesome btw, just one more thing. With the CL description try to: - Keep a ...
2 years ago (2017-09-25 15:57:30 UTC) #31
kjellander_webrtc
On 2017/09/25 15:57:30, hbos wrote: > Awesome btw, just one more thing. With the CL ...
2 years ago (2017-09-25 18:59:54 UTC) #32
ssilkin
On 2017/09/25 15:57:30, hbos wrote: > Awesome btw, just one more thing. With the CL ...
2 years ago (2017-09-26 07:50:25 UTC) #35
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/3011373002/80001
2 years ago (2017-09-28 08:55:04 UTC) #38
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/3011373002/80001
2 years ago (2017-09-28 08:55:44 UTC) #41
commit-bot: I haz the power
2 years ago (2017-09-28 10:35:54 UTC) #44
Message was sent while issue was closed.
Committed patchset #4 (id:80001) as
https://webrtc.googlesource.com/src/+/1440c9f70cdd701210652c7758b664ae82a4fef3

Powered by Google App Engine
This is Rietveld 408576698