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

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

Created:
1 year, 9 months ago by ssilkin
Modified:
1 year, 9 months 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
1 year, 9 months 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 ...
1 year, 9 months 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 ...
1 year, 9 months 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, ...
1 year, 9 months 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 ...
1 year, 9 months 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.
1 year, 9 months ago (2017-09-25 15:08:47 UTC) #29
hbos
LGTM, but don't land this without having the chromium bits sorted out
1 year, 9 months ago (2017-09-25 15:49:34 UTC) #30
hbos
Awesome btw, just one more thing. With the CL description try to: - Keep a ...
1 year, 9 months 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 ...
1 year, 9 months 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 ...
1 year, 9 months 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
1 year, 9 months 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
1 year, 9 months ago (2017-09-28 08:55:44 UTC) #41
commit-bot: I haz the power
1 year, 9 months 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