New issue
Advanced search Search tips

Issue 833762 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Merge to M67: Use codec's bitrate limits if SVC is off

Project Member Reported by ssilkin@chromium.org, Apr 17 2018

Issue description

This fixes the regressions in quality and bitrate metrics which were caused by applying SVC rate limits at encoding in single spatial layer.

Original bug: https://bugs.chromium.org/p/webrtc/issues/detail?id=9151

Webrtc commit: https://webrtc.googlesource.com/src/+/fafeac3517a8c2c702bf3f65dfe4ac0493eb5874

Which was imported to chromium: https://chromium.googlesource.com/chromium/src/+/c42eb293fc9fab232250d9550fc37f0677ede892

 

Comment 1 by gov...@chromium.org, Apr 17 2018

Pls apply appropriate OSs label. Thank you.

Comment 2 by gov...@chromium.org, Apr 17 2018

Pls apply appropriate OSs label. Thank you.

Comment 3 by gov...@chromium.org, Apr 18 2018

Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows

Comment 4 by gov...@chromium.org, Apr 18 2018

Exactly which CL you're requesting a merge for? And is the change well baked in canary, safe to merge?
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 18 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
>>Exactly which CL you're requesting a merge for?
Original CL: https://webrtc-review.googlesource.com/69813
CL that merges the fix to M67: https://webrtc-review.googlesource.com/70380

>>And is the change well baked in canary, safe to merge?
Local testing of M67 with the fix didn't reveal any issues.

Comment 7 by gov...@chromium.org, Apr 18 2018

CL got auto approved at #5. Pls merge ASAP to M67 as per comment #6. Thank you.
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 18 2018

Labels: merge-merged-67
The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/ce137ce112e025218a2d266bcfd994bdb5fe4588

commit ce137ce112e025218a2d266bcfd994bdb5fe4588
Author: Sergey Silkin <ssilkin@webrtc.org>
Date: Wed Apr 18 14:59:18 2018

Merge to M67: Use codec's bitrate limits if SVC is off.

Adding SVC rate allocator and layering configurator caused regression
for VP9 non-SVC senders. SVC bitrate limits, which were supposed to
be used only when spatial layering is enabled, are applied when
encoding single spatial layer. E.g. for VP9 360p sender maximum bitrate
is limited to 500kbps.

This fixes the regression. If sender is configured to send VP9 single
layer then codec's bitrate limits are applied to this layer.

(cherry picked from commit fafeac3517a8c2c702bf3f65dfe4ac0493eb5874)

Bug:  chromium:833762 ,  webrtc:9151 ,  chromium:831093 
Change-Id: Ia1ae4087155ad7917a3443304a21532f1e68ea65
Reviewed-on: https://webrtc-review.googlesource.com/70380
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Michael Horowitz <mhoro@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#22862}
Cr-Commit-Position: refs/branch-heads/67@{#2}
Cr-Branched-From: 4da18e89bdee78df4478b66cdd0e6f6a38d61b4d-refs/heads/master@{#22779}
[modify] https://crrev.com/ce137ce112e025218a2d266bcfd994bdb5fe4588/modules/video_coding/codecs/vp9/svc_rate_allocator.cc
[modify] https://crrev.com/ce137ce112e025218a2d266bcfd994bdb5fe4588/modules/video_coding/video_codec_initializer.cc
[modify] https://crrev.com/ce137ce112e025218a2d266bcfd994bdb5fe4588/modules/video_coding/video_codec_initializer_unittest.cc

Comment 9 by gov...@chromium.org, Apr 18 2018

Per comment #8, this is already merged to M67. Pls remove "Merge-Approved-67" label if nothing else is pending.
Labels: -Merge-Approved-67
Status: Fixed (was: Assigned)
[bulk-edit: disregard if N/A] Can the owner please set milestone to this bug if applicable?
Labels: M-67

Sign in to add a comment