I think we should add logging of received frames to the RTC event log. Please make a note of what values would have helped debug this issue in  https://bugs.chromium.org/p/webrtc/issues/detail?id=8802.

HW enc encodes every 256th frame as intra: frame_num is multiple to 256 in all intra (non-IDR) slices.

Deleted: kip_hw_enc_iframe.png 37.6 KB

	kip_hw_enc_iframe.png 37.6 KB View Download

+1 for the idea in #1.

This is why every 256th frame is intra - 
https://cs.chromium.org/chromium/src/media/gpu/vaapi/vaapi_video_encode_accelerator.cc?rcl=5c576d1e3a035b0ecb05c384b7e21800cfe0ee9f&l=64.

I don't think we need intra frames. Intra is not an entry point for decoding. I suggest to disable periodic intra completely or, if that is not possible, set kIPeriod = kIDRPeriod.

Seems like that parameter might affect HW on a lot of different platforms? In that case I think it would be really good to increase/remove it.

The problem is reproduced with caroline.
Let me try to fix this.

I agreed the performance issue are located at encoder.
If vsc=vp8 is set, the performance is good. In the case of vsc=h264, it is bad.
However, even after I set kIPeriod = kIDRPeriod, the performance was still bad.

The received stream become smooth if disanabling WebRTC video HW encoding. #disable-webrtc-hw-encoding.

Discussing with silkin@, the distribution of IDR frame in the above picture (recv_from_kip_apprtc_h264_200kb.png) is strange.
If a receiver gets such a stream, decoding would take too time to show the stream smoothly on the screen.
Indeed the distribution is not intended with our HW encoder, I will next be sure such a stream is really generated by our HW encoder, vaapi_video_encode_accelerator.

Log the size and NALU type of frames in stream encoded by VaapiVideoEncodeAccelerator.
https://cs.chromium.org/chromium/src/media/gpu/vaapi/vaapi_video_encode_accelerator.cc?sq=package:chromium&dr&l=570

I attached them.
no-op.log is the log of encoded stream when capturing static sight.
waving_hand.log is the log of encoded stream when capturing waving hands.

Except the second IDR frame in the beginning, the encoded stream looks correct.
I frames and IDR frames are generated periodically as expected.

Deleted: no-op.log 1.0 MB

no-op.log 1.0 MB View Download

Deleted: waving_hand.log 1.1 MB

waving_hand.log 1.1 MB View Download

>>I frames and IDR frames are generated periodically as expected.
hiroh, is there any reason to have periodic I-frames?

I guess there is no reason. posciak@ should know.
However, as mentioned above, setting IFramePeriod to IDRPeriod didn't resolve this issue.

I-frames produced by Hw encoder on HP Kip caused quality fluctuations. It is either quality if I-frames is low or, since they are huge, encoder uses high Qp on next frames after I-frame to compensate for buffer overshoot.

RTC doesn't benefit from periodic I-frames. These frames are not recovery frames since it is not guaranteed that P-frame after I-frame won't refer to frames before I-frame.

Please, disable periodic I-frames.

Is disabling periodic I-frame equivalent to setting I frame period to IDR frame?

You need to make sure that encoder produces IDR-frame but not I-frame when both are requested at the same time. In my opinion it would be more safe to completely remove periodic I-frame requests.

I set kIDRPeriod to 2048 * 2048, which I guess equivalent to remove I-frame.
The performance looks still bad.

I didn't try to completely remove I-frame yet.

I think I found what caused frequent IDR frames in the beginning of call. The wrapper request IDR on every bitrate/framerate update - https://cs.chromium.org/chromium/src/media/gpu/vaapi/vaapi_video_encode_accelerator.cc?rcl=9d05ec002dd34b7cb4b9d661aed08bbc70a7959d&l=773

Is this really needed?

Re #21, that was my observation too, see https://bugs.chromium.org/p/chromium/issues/detail?id=801493#c33.holmer@ told this is expected from webrtc side. Then, we can look into ignoring these on the HW wncoder side. I didn't get a clear feedback there from HW encode owners though.

I removed I frame completely and the periodical lower quality stream was gone.

This is the original stream , where I frame comes every 256 frames.
https://screencast.googleplex.com/cast/NTcxNzA0OTU2NTI0OTUzNnwxMWJjZjcwNS02Yw

This is the stream after removing I frame completely.
https://screencast.googleplex.com/cast/NTc0MzM1MzgyNDAxODQzMnw4ZGQ3ODMyNy1kMQ

I feel a difference.  What do you think?

The difference is quite noticeable.

I created separate ticket on frequent IDRs - https://bugs.chromium.org/p/chromium/issues/detail?id=833760.

Create the CL to remove I frame on encode stream.
https://chromium-review.googlesource.com/c/chromium/src/+/1013449

ssilkin@ Hiro has a CL to remove I Frame. How do we add a test for this? Can we use psnr or ssim to test this?

It would be enough to have functional test which verifies that encoder issues only IDR and P frames and that periodicity of IDRs is 2048 frames.

Re#29:
- H264 HW encode is also used for video recording in ChromeOS camera app. Are removing I frame and IDR every 2048 frame also OK for recorded videos?
- Is this functional test expected on all VEAs? VEA API do not specify about this behavior. I'm not sure about the behaviors of other VEAs. We need to decide where to put the test. Ideally it should be in video_encode_accelerator_unittest.cc and run on all VEAs. If it's specific to ChromeOS, we can put that in CrOS autotest.
-Is there a tolerance (like an acceptable range) for IDR every 2048 frame? For example, is it OK to get IDR every 2000 or 21000 frames? I'm wondering if other ARM CrOS devices will pass the test.

> H264 HW encode is also used for video recording in ChromeOS camera app. Are removing I frame and IDR every 2048 frame also OK for recorded videos?
I am assuming that is using MediaRecorder. We force a keyframe in every 100 frame for that, so it shouldn't be a problem.
https://cs.chromium.org/chromium/src/content/renderer/media_recorder/vea_encoder.cc?q=vea_encoder&sq=package:chromium&dr=C&l=33

I frames are also useful in order to recover from transmission issues. Without them, we are depending on every single frame since the beginning of the broadcast to have been transmitted perfectly. Is this assumption realistic in the current scenario?

Oh, looks like I jumped a bit too fast. The encoder is apparently still emitting IDR frames with the proposed change.

I rebased the CL https://chromium-review.googlesource.com/c/chromium/src/+/1013449
It will hopefully be merged soon.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5594e40b93c04b28a314535ed9065ae734d9e81b

commit 5594e40b93c04b28a314535ed9065ae734d9e81b
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Tue Jun 05 06:13:59 2018

media/gpu/vaapi H264Encoder: Remove I frame completely from generated encoded H264 stream

The size of I frame is large and thus is likely to make other frames lower
quality on the limited bitrate in webrtc case.
Originally, H264Encoder generates I frame every 248 frames. Therefore, webrtc
receiver observes low quality stream every 8 seconds.
Not generating I frame doesn't matter. This changes as H264Encoder doesn't generate
I frame on encoded stream.

BUG= chromium:806235 
TEST=VEA unittest on caroline
TEST=https://appr.tc/?vrc=h264&vrbr=200&vsc=h264&vsbr=200
TEST=Record video with Chrome Camera App

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I929f943659d2d547afc192a7a354ccaad398d576
Reviewed-on: https://chromium-review.googlesource.com/1013449
Reviewed-by: Pawel Osciak <posciak@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564374}
[modify] https://crrev.com/5594e40b93c04b28a314535ed9065ae734d9e81b/media/gpu/vaapi/h264_encoder.cc

[bulk-edit: disregard if N/A] Can the owner please set milestone to this bug if applicable?

#comment37, the CL for this issue was already landed on canary Chrome OS and thus it will be released as M69.