Is this something we want to implement? It's a simple change but I'd like to hear from the owners of these components in chromium before implementing it.

Yes, I think it's up to the encoder owner to decide on this. +emircan for input.

I don't disagree with the point its upto to the encoder owner. However, we also need to take into account the impact of not supporting it. The webrtc stack does not have a reasonable fallback when encoder implementation doesn't opt for this and the overall call quality is bad. I see two possibilities in here
1. Encoders always opt in
2. Implement a static quality scaler as a fallback to qp based scaler. This can be based on the sender bandwidth estimate (like the way its done during the initial ramp up phase where there is a bitrate to resolution mapping enforced if encoded frames get dropped due to bitrate constraints). I am referring to this.

uint32_t MaximumFrameSizeForBitrate(uint32_t kbps) {
  if (kbps > 0) {
    if (kbps < 300 /* qvga */) {
      return 320 * 240;
    } else if (kbps < 500 /* vga */) {
      return 640 * 480;
    }
  }
  return std::numeric_limits<uint32_t>::max();
}

Not an optimal solution but a reasonable fallback.

Re bitrate adjustment problem, VideoEncoder::SetRates() is implemented in the underlying HW encoders. When there is a change in bitrate, this function can be called with the updated value and encoder output should adapt to it within a couple seconds. Additionally, we have custom BitrateAdjuster for Mac to handle this more smoothly.

Adjusting the resolution can be done by restarting the underlying HW encoder(RTCVideoEncoder) as HW encode sessions are tied to a fixed input resolution. This already happens for HW encoders when there is need for cpu adaptation-when googEncodeMs is too high-.

Re qp values, they are not reported by HW encoder. I just checked it has been like that since CrOS HW encoders were the only implementations. AFAICT, they can be calculated by parsing through PPS if HW encoder fills those fields. It can even be done within webrtc for all incoming H264 encoded data. +cc posciak to comment on that. 

https://cs.chromium.org/chromium/src/third_party/webrtc/common_video/h264/h264_bitstream_parser.h?rcl=2149a54ecafd877efdc40e805e4492519b582562&l=47

It's easy to add WebRTC's QP scaling facilities to the HW encoders, we just need to override a method on RTCvideoEncoder.

As for the QP values, in the WebRTC encoder implementations we parse them from the bitstream in the encoder. The reason for this is that we don't want to waste cycles parsing the bitstream if a) the qp can be provided by the encoder implementation or b) the qp value won't be used because the encoder has not opted in to qp scaling. Here[1] is the relevant code from our VideoToolbox encoder.

[1]: https://cs.chromium.org/chromium/src/third_party/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.mm?type=cs&q=videotoolbox/&l=730

Hi kthelgason/emircan,

We have tried a fix for this issue on Chromium master branch and it seems to be working. Attaching the patch file, please let us know if this fix looks good. Thanks.

Deleted: chromium-726001.diff 2.8 KB

chromium-726001.diff 2.8 KB Download

Thanks for the work hiitz@. You can put a patch up for review and we can add comments on code. I am not sure how threshold should be defined. kthelgason@ can you point me to a doc if this is standardized? posciak@ what do you think about this being common across HW encode platforms?

Hi emircan,

Sure, we will be submitting a pull request for this patch. As per steps listed here https://www.chromium.org/developers/contributing-code, we have to sign corporate license agreement (assuming it is mandatory) and hence it will take few days before we can create a pull request.

Also, I would like to inform that the patch works on both Windows and Mac systems.

Thanks,
Hitesh

Ideally I think it would be great to plug this through to VEAs to (optionally) allow the HW encoder to return these values without having to parse again in RTCVE, and only do so if it wasn't provided. We already have these values in at least some of the HW encoders and they could be provided.

Also, would perhaps webrtc VEA client be a better location to do this, instead of RTCVE?

I'm quite sure quality scaler doesn't kick in for software encoding as well.

The bug I filed earlier may be merged into this one, I guess:
https://bugs.chromium.org/p/chromium/issues/detail?id=712155

Hi emircan/posciak,

We have observed H264 hardware encoder failure while scaling down resolution to 480x270. From HD down-scale to 960x540 and 640x360 works fine, but third attempt to down-scale fails. With our QP scaling fix, we are able to reproduce this issue. We reverted our fix and tried down-scaling using CPU overuse detection which was found to be limited to only two attempts of down-scaling as per https://cs.chromium.org/chromium/src/third_party/webrtc/video/vie_encoder.h?l=72.
But then increasing the limit (setting kMaxCpuResolutionDowngrades to 4) also led to encoder failure with CPU overuse detection for lower resolutions.
Below are the logs for this observation using CPU overuse detection with some changes to trigger it aggressively(lower values for CPU usage threshold):
[16910:50183:0720/144325.733022:INFO:vie_encoder.cc(249)] Scaling down resolution, max pixels: 138240 
[16910:50183:0720/144325.733057:INFO:vie_encoder.cc(1010)] Downgrade counts: fps: {quality:0 cpu:0}, resolution: {quality:0 cpu:3}
[16910:50183:0720/144325.733075:VERBOSE1:overuse_frame_detector.cc(536)]  Frame stats:  encode usage 99 overuse detections 3 rampup delay 40000
[16910:40963:0720/144325.760872:INFO:videoadapter.cc(245)] Frame size changed: scaled 1500 / out 2253 / in 2253 Changes: 3 Input: 1280x720 Scale: 3/8 Output: 480x270 i0
[16910:50183:0720/144325.761245:INFO:vie_encoder.cc(765)] Video frame parameters changed: dimensions=480x270, texture=1.
[16910:50183:0720/144325.773878:WARNING:videoencodersoftwarefallbackwrapper.cc(36)] Encoder requesting fallback to codec not supported in software.
[16910:50183:0720/144325.773924:ERROR:generic_encoder.cc(63)] Failed to initialize the encoder associated with payload name: H264
[16910:50183:0720/144325.773956:ERROR:codec_database.cc(282)] Failed to initialize video encoder.
[16910:50183:0720/144325.774046:ERROR:video_sender.cc(93)] Failed to initialize set encoder with payload name 'H264'.
[16910:50183:0720/144325.774077:ERROR:vie_encoder.cc(613)] Failed to configure encoder. 

Is there any known issue related to not supporting lower resolutions for H264 hardware encoder in the master branch?

Thanks,
Hitesh

Note : Tested with laptop running macOS Sierra version 10.12.5

hitz:

Yes, the hardware codecs often don't support arbitrary resolutions and we've seen quite often that 480x270 appears not to work. Often resolutions that are even multiples of 4 or 8 work better, depending on the specific hardware.

It's possible to pass a required_alignment to an AdaptedVideoTrackSource to ensure that it will scale down to multiples of a given number. See here[1].

[1]: https://cs.chromium.org/chromium/src/third_party/webrtc/media/base/adaptedvideotracksource.h?sq=package:chromium&l=33

kthelgason,

Is it fine to introduce a variable like kMaxCpuResolutionDowngrades (https://cs.chromium.org/chromium/src/third_party/webrtc/video/vie_encoder.h?l=72) to limit resolution down scale beyond two attempts for QP scaling as well?

Thanks,
Hitesh

I'd prefer not to add that in webrtc (in fact it was recently removed) because we want to be able to scale down to low resolutions in low-bw scenarios.

However, the encoder could ignore requests to scale down further than some limit, if it doesn't support arbitrary resolutions. The quality scaler will only try to scale down if the last request succeeded, so that will prevent repeated scale down requests to the encoder.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Is this still an issue? It sounds like it could be a good thing to fix.
asapersson@ wdyt?

This is still an issue. Yes. I tried this on chrome 69 and I am able to recreate the problem

It is possible to set the minimum resolution to scale down to (see min_pixels_per_frame in ScalingSettings):
https://cs.chromium.org/chromium/src/third_party/webrtc/api/video_codecs/video_encoder.h?l=111