RTC encoder/decoder factory support for H.265 behind flag. [chromium/src : main]

3 views
Skip to first unread message

Jianlin Qiu (Gerrit)

unread,
Apr 17, 2024, 9:15:49 AMApr 17
to Sergey Silkin, Sergey Silkin, Dale Curtis, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
Attention needed from Dale Curtis, Ilya Nikolaevskiy, Philip Eliasson, Sergey Silkin and Sergey Silkin

Jianlin Qiu added 6 comments

File media/webrtc/webrtc_features.h
Line 47, Patchset 10:BASE_DECLARE_FEATURE(kExperimentalWebRtcH265);
Sergey Silkin . resolved

Let's make naming consistent with other WebRTC related features. kWebRtcAllowH265 ?

Jianlin Qiu

How do you think about we have 2 flags for this:
kWebRtcAllowH265Send
kWebRtcAllowH265Receive

I would assume we probably does not have the intention to allow encode by default, but the capability of receiving H.265 streams improves interop with native applications in the future?

Sergey Silkin

Yes, I think it makes sense to split the flag into send/recv.

Jianlin Qiu

Done

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc
Line 520, Patchset 10: if (!resolution_monitor && config.codec() != media::VideoCodec::kHEVC) {
Sergey Silkin . resolved

Should HasSoftwareFallback() be used here (and in other similar places) instead?

Jianlin Qiu

Done

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory.cc
Line 51, Patchset 10:// To support H.265 level 3.1, the default size is set to 1920x1080.
Sergey Silkin . resolved

Sounds slightly misleading. Unlike in the case of H264 there is no SW decoder we need to align max supported level with. I guess we simply want to signal support of 1080p and derive necessary min level for this resolution.

Jianlin Qiu

Done

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory_test.cc
Line 241, Patchset 10: true /*reference_scaling*/),
Sergey Silkin . resolved

should this be false?

Jianlin Qiu

Done

File third_party/blink/renderer/platform/peerconnection/rtc_video_encoder.cc
Line 1938, Patchset 10: if ((profile_ < media::HEVCPROFILE_MIN ||
profile_ > media::HEVCPROFILE_MAX) &&
Sergey Silkin . unresolved

Any reason not to use codec_settings->codecType here?

Jianlin Qiu

No specific reason. I'm just following the code at line 1953 & 1954 for consistency.

File third_party/blink/renderer/platform/peerconnection/rtc_video_encoder_factory.cc
Line 35, Patchset 10:#if BUILDFLAG(RTC_USE_H265)

#endif // BUILDFLAG(RTC_USE_H265)
Sergey Silkin . resolved

remove?

Jianlin Qiu

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Dale Curtis
  • Ilya Nikolaevskiy
  • Philip Eliasson
  • Sergey Silkin
  • Sergey Silkin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
Gerrit-Change-Number: 5456050
Gerrit-PatchSet: 12
Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
Gerrit-Reviewer: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Reviewer: Philip Eliasson <phil...@webrtc.org>
Gerrit-Reviewer: Sergey Silkin <ssi...@webrtc.org>
Gerrit-CC: Erik Språng <spr...@chromium.org>
Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
Gerrit-CC: Sergey Silkin <ssi...@chromium.org>
Gerrit-Attention: Dale Curtis <dalec...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@webrtc.org>
Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
Gerrit-Comment-Date: Wed, 17 Apr 2024 13:15:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jianlin Qiu <jianl...@intel.com>
Comment-In-Reply-To: Sergey Silkin <ssi...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Dale Curtis (Gerrit)

unread,
Apr 17, 2024, 1:24:32 PMApr 17
to Jianlin Qiu, Sergey Silkin, Sergey Silkin, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
Attention needed from Ilya Nikolaevskiy, Jianlin Qiu, Philip Eliasson, Sergey Silkin and Sergey Silkin

Dale Curtis added 6 comments

Patchset-level comments
File-level comment, Patchset 12 (Latest):
Dale Curtis . resolved

Defer to RTC owners for approval.

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc
Line 107, Patchset 12 (Latest):bool HasSoftwareFallback(media::VideoCodec video_codec) {
Dale Curtis . unresolved

Maybe clearer just as:

```
if (video_codec == HEVC) {
return false;
}
```

outside of the buildflags.

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory.cc
Line 29, Patchset 12 (Latest):#if BUILDFLAG(RTC_USE_H265)
Dale Curtis . unresolved

Move build flag includes below others generally.

Line 48, Patchset 12 (Latest):const int kDefaultFps = 30;
Dale Curtis . unresolved

constexpr all these.

File third_party/blink/renderer/platform/peerconnection/rtc_video_encoder_factory.cc
Line 22, Patchset 12 (Latest):#if BUILDFLAG(RTC_USE_H265)
Dale Curtis . unresolved

Move below other includes.

Line 235, Patchset 12 (Latest): SupportedFormats supported_h265_formats;
Dale Curtis . unresolved

You could choose something more generic like `low_priority_formats` so more of the code doesn't need to be behind a buildflag.

Open in Gerrit

Related details

Attention is currently required from:
  • Ilya Nikolaevskiy
  • Jianlin Qiu
Gerrit-Attention: Sergey Silkin <ssi...@webrtc.org>
Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
Gerrit-Comment-Date: Wed, 17 Apr 2024 17:22:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Jianlin Qiu (Gerrit)

unread,
Apr 17, 2024, 9:57:14 PMApr 17
to Sergey Silkin, Sergey Silkin, Dale Curtis, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
Attention needed from Dale Curtis, Ilya Nikolaevskiy, Philip Eliasson, Sergey Silkin and Sergey Silkin

Jianlin Qiu added 5 comments

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc
Line 107, Patchset 12:bool HasSoftwareFallback(media::VideoCodec video_codec) {
Dale Curtis . resolved

Maybe clearer just as:

```
if (video_codec == HEVC) {
return false;
}
```

outside of the buildflags.

Jianlin Qiu

Done

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory.cc
Line 29, Patchset 12:#if BUILDFLAG(RTC_USE_H265)
Dale Curtis . resolved

Move build flag includes below others generally.

Jianlin Qiu

Done

Line 48, Patchset 12:const int kDefaultFps = 30;
Dale Curtis . resolved

constexpr all these.

Jianlin Qiu

Done

File third_party/blink/renderer/platform/peerconnection/rtc_video_encoder_factory.cc
Line 22, Patchset 12:#if BUILDFLAG(RTC_USE_H265)
Dale Curtis . resolved

Move below other includes.

Jianlin Qiu

Done

Line 235, Patchset 12: SupportedFormats supported_h265_formats;
Dale Curtis . resolved

You could choose something more generic like `low_priority_formats` so more of the code doesn't need to be behind a buildflag.

Jianlin Qiu

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Dale Curtis
  • Ilya Nikolaevskiy
  • Philip Eliasson
  • Sergey Silkin
  • Sergey Silkin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
Gerrit-Change-Number: 5456050
Gerrit-PatchSet: 13
Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
Gerrit-Reviewer: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Reviewer: Philip Eliasson <phil...@webrtc.org>
Gerrit-Reviewer: Sergey Silkin <ssi...@webrtc.org>
Gerrit-CC: Erik Språng <spr...@chromium.org>
Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
Gerrit-CC: Sergey Silkin <ssi...@chromium.org>
Gerrit-Attention: Dale Curtis <dalec...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@webrtc.org>
Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
Gerrit-Comment-Date: Thu, 18 Apr 2024 01:57:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Dale Curtis <dalec...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Sergey Silkin (Gerrit)

unread,
Apr 19, 2024, 6:46:32 AMApr 19
to Jianlin Qiu, Sergey Silkin, Dale Curtis, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
Attention needed from Ilya Nikolaevskiy, Jianlin Qiu, Philip Eliasson and Sergey Silkin

Sergey Silkin added 2 comments

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory_test.cc
Line 188, Patchset 14 (Latest):#if BUILDFLAG(RTC_USE_H265)
Sergey Silkin . unresolved

Seems that this #if is not really needed.

File third_party/blink/renderer/platform/peerconnection/rtc_video_encoder.cc
Line 1938, Patchset 10: if ((profile_ < media::HEVCPROFILE_MIN ||
profile_ > media::HEVCPROFILE_MAX) &&
Sergey Silkin . unresolved

Any reason not to use codec_settings->codecType here?

Jianlin Qiu

No specific reason. I'm just following the code at line 1953 & 1954 for consistency.

Sergey Silkin

Profile-based conditions were added in https://chromium-review.googlesource.com/c/chromium/src/+/3351622. The tests added in the same CL explicitly set codecType. For example: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/peerconnection/rtc_video_encoder_test.cc;l=802-805

Not sure why profile was used. codecType seems to be a better option in both places.

Open in Gerrit

Related details

Attention is currently required from:
  • Ilya Nikolaevskiy
  • Jianlin Qiu
  • Philip Eliasson
  • Sergey Silkin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
Gerrit-Change-Number: 5456050
Gerrit-PatchSet: 14
Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
Gerrit-Reviewer: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Reviewer: Philip Eliasson <phil...@webrtc.org>
Gerrit-Reviewer: Sergey Silkin <ssi...@webrtc.org>
Gerrit-CC: Erik Språng <spr...@chromium.org>
Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
Gerrit-CC: Sergey Silkin <ssi...@chromium.org>
Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
Gerrit-Comment-Date: Fri, 19 Apr 2024 10:46:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Jianlin Qiu (Gerrit)

unread,
Apr 19, 2024, 10:01:23 AMApr 19
to Sergey Silkin, Sergey Silkin, Dale Curtis, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
Attention needed from Ilya Nikolaevskiy, Philip Eliasson, Sergey Silkin and Sergey Silkin

Jianlin Qiu added 2 comments

File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory_test.cc
Line 188, Patchset 14:#if BUILDFLAG(RTC_USE_H265)
Sergey Silkin . resolved

Seems that this #if is not really needed.

Jianlin Qiu

Done

File third_party/blink/renderer/platform/peerconnection/rtc_video_encoder.cc
Line 1938, Patchset 10: if ((profile_ < media::HEVCPROFILE_MIN ||
profile_ > media::HEVCPROFILE_MAX) &&
Sergey Silkin . resolved

Any reason not to use codec_settings->codecType here?

Jianlin Qiu

No specific reason. I'm just following the code at line 1953 & 1954 for consistency.

Sergey Silkin

Profile-based conditions were added in https://chromium-review.googlesource.com/c/chromium/src/+/3351622. The tests added in the same CL explicitly set codecType. For example: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/peerconnection/rtc_video_encoder_test.cc;l=802-805

Not sure why profile was used. codecType seems to be a better option in both places.

Jianlin Qiu

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Ilya Nikolaevskiy
  • Philip Eliasson
  • Sergey Silkin
  • Sergey Silkin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
Gerrit-Change-Number: 5456050
Gerrit-PatchSet: 15
Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
Gerrit-Reviewer: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Reviewer: Philip Eliasson <phil...@webrtc.org>
Gerrit-Reviewer: Sergey Silkin <ssi...@webrtc.org>
Gerrit-CC: Erik Språng <spr...@chromium.org>
Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
Gerrit-CC: Sergey Silkin <ssi...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@webrtc.org>
Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
Gerrit-Comment-Date: Fri, 19 Apr 2024 14:01:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sergey Silkin <ssi...@webrtc.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Sergey Silkin (Gerrit)

unread,
Apr 19, 2024, 10:17:11 AMApr 19
to Jianlin Qiu, Chromium LUCI CQ, Sergey Silkin, Dale Curtis, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
Attention needed from Ilya Nikolaevskiy, Jianlin Qiu, Philip Eliasson and Sergey Silkin

Sergey Silkin voted Code-Review+1

Code-Review+1
Open in Gerrit

Related details

Attention is currently required from:
  • Ilya Nikolaevskiy
  • Jianlin Qiu
  • Philip Eliasson
  • Sergey Silkin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
Gerrit-Change-Number: 5456050
Gerrit-PatchSet: 15
Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
Gerrit-Reviewer: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Reviewer: Jianlin Qiu <jianl...@intel.com>
Gerrit-Reviewer: Philip Eliasson <phil...@webrtc.org>
Gerrit-Reviewer: Sergey Silkin <ssi...@chromium.org>
Gerrit-Reviewer: Sergey Silkin <ssi...@webrtc.org>
Gerrit-CC: Erik Språng <spr...@chromium.org>
Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
Gerrit-Attention: Sergey Silkin <ssi...@webrtc.org>
Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
Gerrit-Comment-Date: Fri, 19 Apr 2024 14:16:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Jianlin Qiu (Gerrit)

unread,
Apr 19, 2024, 12:47:59 PMApr 19
to Daniel Cheng, Sergey Silkin, Chromium LUCI CQ, Sergey Silkin, Dale Curtis, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
Attention needed from Dale Curtis, Daniel Cheng, Harald Alvestrand, Ilya Nikolaevskiy, Jianlin Qiu, Philip Eliasson, Sergey Silkin and Sergey Silkin

Message from Jianlin Qiu

Set Ready For Review

Open in Gerrit

Related details

Attention is currently required from:
  • Dale Curtis
  • Daniel Cheng
  • Harald Alvestrand
  • Ilya Nikolaevskiy
  • Jianlin Qiu
  • Philip Eliasson
  • Sergey Silkin
  • Sergey Silkin
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
    Gerrit-Change-Number: 5456050
    Gerrit-PatchSet: 19
    Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
    Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
    Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
    Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
    Gerrit-Reviewer: Ilya Nikolaevskiy <il...@chromium.org>
    Gerrit-Reviewer: Jianlin Qiu <jianl...@intel.com>
    Gerrit-Reviewer: Philip Eliasson <phil...@webrtc.org>
    Gerrit-Reviewer: Sergey Silkin <ssi...@chromium.org>
    Gerrit-Reviewer: Sergey Silkin <ssi...@webrtc.org>
    Gerrit-CC: Erik Språng <spr...@chromium.org>
    Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
    Gerrit-Attention: Dale Curtis <dalec...@chromium.org>
    Gerrit-Attention: Sergey Silkin <ssi...@webrtc.org>
    Gerrit-Attention: Daniel Cheng <dch...@chromium.org>
    Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
    Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
    Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
    Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
    Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
    Gerrit-Comment-Date: Fri, 19 Apr 2024 16:47:48 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Dale Curtis (Gerrit)

    unread,
    Apr 19, 2024, 4:05:19 PMApr 19
    to Jianlin Qiu, Mirko Bonadei, Daniel Cheng, Sergey Silkin, Chromium LUCI CQ, Sergey Silkin, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
    Attention needed from Daniel Cheng, Harald Alvestrand, Ilya Nikolaevskiy, Jianlin Qiu, Mirko Bonadei, Philip Eliasson, Sergey Silkin and Sergey Silkin

    Dale Curtis added 1 comment

    Patchset-level comments
    File-level comment, Patchset 19 (Latest):
    Dale Curtis . unresolved

    Jianlin, can you pare down to just the required reviewers?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Daniel Cheng
    • Harald Alvestrand
    • Ilya Nikolaevskiy
    • Jianlin Qiu
    • Mirko Bonadei
    • Philip Eliasson
    • Sergey Silkin
    • Sergey Silkin
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
        Gerrit-Change-Number: 5456050
        Gerrit-PatchSet: 19
        Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
        Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
        Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
        Gerrit-Reviewer: Ilya Nikolaevskiy <il...@chromium.org>
        Gerrit-Reviewer: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Reviewer: Philip Eliasson <phil...@webrtc.org>
        Gerrit-Reviewer: Sergey Silkin <ssi...@chromium.org>
        Gerrit-Reviewer: Sergey Silkin <ssi...@webrtc.org>
        Gerrit-CC: Erik Språng <spr...@chromium.org>
        Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
        Gerrit-Attention: Sergey Silkin <ssi...@webrtc.org>
        Gerrit-Attention: Daniel Cheng <dch...@chromium.org>
        Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
        Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
        Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Attention: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
        Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
        Gerrit-Comment-Date: Fri, 19 Apr 2024 20:05:06 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Daniel Cheng (Gerrit)

        unread,
        Apr 19, 2024, 4:12:57 PMApr 19
        to Jianlin Qiu, Mirko Bonadei, Daniel Cheng, Sergey Silkin, Chromium LUCI CQ, Sergey Silkin, Dale Curtis, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Harald Alvestrand, Ilya Nikolaevskiy, Jianlin Qiu, Mirko Bonadei, Philip Eliasson, Sergey Silkin and Sergey Silkin

        Daniel Cheng added 2 comments

        File third_party/blink/renderer/platform/peerconnection/DEPS
        Line 59, Patchset 19 (Latest): "rtc_video_encoder_factory.cc": [
        Daniel Cheng . unresolved

        Probably easier to just add this in include rules rather than specific include rules, unless there's a specific reason to scope this?

        File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory.cc
        Line 62, Patchset 19 (Latest):constexpr std::array<CodecConfig, 11> kCodecConfigs = {{
        Daniel Cheng . unresolved

        This std::array needs to be sized according to how many elements are in it.

        Unfortunately, CTAD requires specifying all or nothing.

        We can:

        • use a regular C array, i.e. constexpr CodecConfig kCodecConfigs[] = {{ ... }};
        • or we can try to dynamically set the size here.

        I would probably just use a regular C array.

        Open in Gerrit

        Related details

        Attention is currently required from:
        Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
        Gerrit-Attention: Philip Eliasson <phil...@webrtc.org>
        Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Attention: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Attention: Ilya Nikolaevskiy <il...@chromium.org>
        Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
        Gerrit-Comment-Date: Fri, 19 Apr 2024 20:12:45 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Daniel Cheng (Gerrit)

        unread,
        Apr 19, 2024, 4:19:07 PMApr 19
        to Jianlin Qiu, Mirko Bonadei, Daniel Cheng, Sergey Silkin, Chromium LUCI CQ, Sergey Silkin, Dale Curtis, Harald Alvestrand, Ilya Nikolaevskiy, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Harald Alvestrand, Ilya Nikolaevskiy, Jianlin Qiu, Mirko Bonadei, Philip Eliasson, Sergey Silkin and Sergey Silkin

        Daniel Cheng added 1 comment

        File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory.cc
        Line 62, Patchset 19 (Latest):constexpr std::array<CodecConfig, 11> kCodecConfigs = {{
        Daniel Cheng . unresolved

        This std::array needs to be sized according to how many elements are in it.

        Unfortunately, CTAD requires specifying all or nothing.

        We can:

        • use a regular C array, i.e. constexpr CodecConfig kCodecConfigs[] = {{ ... }};
        • or we can try to dynamically set the size here.

        I would probably just use a regular C array.

        Daniel Cheng

        Alternatively a CTAD variant:

        ```
        constexpr std::array kCodecConfigs = {
        CodecConfig{...},
        CodecConfig{...},
        ...
        };
        ```
        Gerrit-Comment-Date: Fri, 19 Apr 2024 20:18:56 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Daniel Cheng <dch...@chromium.org>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Jianlin Qiu (Gerrit)

        unread,
        Apr 19, 2024, 7:31:35 PMApr 19
        to Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Mirko Bonadei, Daniel Cheng, Sergey Silkin, Chromium LUCI CQ, Sergey Silkin, Dale Curtis, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Daniel Cheng, Mirko Bonadei, Sergey Silkin and Sergey Silkin

        Jianlin Qiu added 2 comments

        File third_party/blink/renderer/platform/peerconnection/DEPS
        Line 59, Patchset 19: "rtc_video_encoder_factory.cc": [
        Daniel Cheng . resolved

        Probably easier to just add this in include rules rather than specific include rules, unless there's a specific reason to scope this?

        Jianlin Qiu

        Done

        File third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory.cc
        Line 62, Patchset 19:constexpr std::array<CodecConfig, 11> kCodecConfigs = {{
        Daniel Cheng . resolved

        This std::array needs to be sized according to how many elements are in it.

        Unfortunately, CTAD requires specifying all or nothing.

        We can:

        • use a regular C array, i.e. constexpr CodecConfig kCodecConfigs[] = {{ ... }};
        • or we can try to dynamically set the size here.

        I would probably just use a regular C array.

        Daniel Cheng

        Alternatively a CTAD variant:

        ```
        constexpr std::array kCodecConfigs = {
        CodecConfig{...},
        CodecConfig{...},
        ...
        };
        ```
        Jianlin Qiu

        Done

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Daniel Cheng
        • Mirko Bonadei
        • Sergey Silkin
        • Sergey Silkin
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
        Gerrit-Change-Number: 5456050
        Gerrit-PatchSet: 20
        Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
        Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
        Gerrit-Reviewer: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Reviewer: Sergey Silkin <ssi...@chromium.org>
        Gerrit-Reviewer: Sergey Silkin <ssi...@webrtc.org>
        Gerrit-CC: Erik Språng <spr...@chromium.org>
        Gerrit-CC: Harald Alvestrand <h...@chromium.org>
        Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
        Gerrit-CC: Ilya Nikolaevskiy <il...@chromium.org>
        Gerrit-CC: Philip Eliasson <phil...@webrtc.org>
        Gerrit-Attention: Sergey Silkin <ssi...@webrtc.org>
        Gerrit-Attention: Daniel Cheng <dch...@chromium.org>
        Gerrit-Attention: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Attention: Sergey Silkin <ssi...@chromium.org>
        Gerrit-Comment-Date: Fri, 19 Apr 2024 23:31:26 +0000
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Sergey Silkin (Gerrit)

        unread,
        Apr 22, 2024, 3:19:18 AMApr 22
        to Jianlin Qiu, Sergey Silkin, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Mirko Bonadei, Daniel Cheng, Chromium LUCI CQ, Dale Curtis, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Daniel Cheng, Mirko Bonadei and Sergey Silkin

        Sergey Silkin removed Sergey Silkin from this change

        Deleted Reviewers:
        • Sergey Silkin
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Daniel Cheng
        • Mirko Bonadei
        • Sergey Silkin
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: deleteReviewer
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
        Gerrit-Change-Number: 5456050
        Gerrit-PatchSet: 20
        Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
        Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
        Gerrit-Reviewer: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Reviewer: Sergey Silkin <ssi...@chromium.org>
        Gerrit-CC: Erik Språng <spr...@chromium.org>
        Gerrit-CC: Harald Alvestrand <h...@chromium.org>
        Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
        Gerrit-CC: Ilya Nikolaevskiy <il...@chromium.org>
        Gerrit-CC: Philip Eliasson <phil...@webrtc.org>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Sergey Silkin (Gerrit)

        unread,
        Apr 22, 2024, 3:19:38 AMApr 22
        to Jianlin Qiu, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Mirko Bonadei, Daniel Cheng, Chromium LUCI CQ, Dale Curtis, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Daniel Cheng, Jianlin Qiu and Mirko Bonadei

        Sergey Silkin voted Code-Review+1

        Code-Review+1
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Daniel Cheng
        • Jianlin Qiu
        • Mirko Bonadei
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
        Gerrit-Change-Number: 5456050
        Gerrit-PatchSet: 20
        Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
        Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
        Gerrit-Reviewer: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Reviewer: Sergey Silkin <ssi...@chromium.org>
        Gerrit-CC: Erik Språng <spr...@chromium.org>
        Gerrit-CC: Harald Alvestrand <h...@chromium.org>
        Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
        Gerrit-CC: Ilya Nikolaevskiy <il...@chromium.org>
        Gerrit-CC: Philip Eliasson <phil...@webrtc.org>
        Gerrit-Attention: Daniel Cheng <dch...@chromium.org>
        Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Attention: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Comment-Date: Mon, 22 Apr 2024 07:19:24 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Daniel Cheng (Gerrit)

        unread,
        Apr 22, 2024, 1:56:45 PMApr 22
        to Jianlin Qiu, Daniel Cheng, Sergey Silkin, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Mirko Bonadei, Chromium LUCI CQ, Dale Curtis, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Jianlin Qiu and Mirko Bonadei

        Daniel Cheng voted and added 1 comment

        Votes added by Daniel Cheng

        Code-Review+1

        1 comment

        Patchset-level comments
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Jianlin Qiu
        • Mirko Bonadei
        Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Attention: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Comment-Date: Mon, 22 Apr 2024 17:56:31 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Jianlin Qiu (Gerrit)

        unread,
        Apr 23, 2024, 3:50:23 AMApr 23
        to Daniel Cheng, Sergey Silkin, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Mirko Bonadei, Chromium LUCI CQ, Dale Curtis, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Dale Curtis and Mirko Bonadei

        Jianlin Qiu added 1 comment

        Patchset-level comments
        Jianlin Qiu . resolved

        Hi, dalecurtis@ could you please help to review */webrtc_features.*,
        mbonadei@ could you please help to review webrtc_overries/BUILD.gn? Thanks!

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Dale Curtis
        • Mirko Bonadei
        Gerrit-Attention: Dale Curtis <dalec...@chromium.org>
        Gerrit-Attention: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Comment-Date: Tue, 23 Apr 2024 07:50:08 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Mirko Bonadei (Gerrit)

        unread,
        Apr 24, 2024, 11:13:22 AMApr 24
        to Jianlin Qiu, Daniel Cheng, Sergey Silkin, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Chromium LUCI CQ, Dale Curtis, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Dale Curtis and Jianlin Qiu

        Mirko Bonadei voted and added 1 comment

        Votes added by Mirko Bonadei

        Code-Review+1

        1 comment

        Patchset-level comments
        Jianlin Qiu . resolved

        Hi, dalecurtis@ could you please help to review */webrtc_features.*,
        mbonadei@ could you please help to review webrtc_overries/BUILD.gn? Thanks!

        Mirko Bonadei

        mbonadei@ could you please help to review webrtc_overries/BUILD.gn? Thanks!

        LGTM for webrtc_overries.

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Dale Curtis
        • Jianlin Qiu
        Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Comment-Date: Wed, 24 Apr 2024 15:13:03 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        Comment-In-Reply-To: Jianlin Qiu <jianl...@intel.com>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Dale Curtis (Gerrit)

        unread,
        Apr 24, 2024, 3:48:59 PMApr 24
        to Jianlin Qiu, Mirko Bonadei, Daniel Cheng, Sergey Silkin, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Chromium LUCI CQ, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Jianlin Qiu

        Dale Curtis voted Code-Review+1

        Code-Review+1
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Jianlin Qiu
        Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Comment-Date: Wed, 24 Apr 2024 19:48:45 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Dale Curtis (Gerrit)

        unread,
        Apr 24, 2024, 3:49:04 PMApr 24
        to Jianlin Qiu, Mirko Bonadei, Daniel Cheng, Sergey Silkin, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Chromium LUCI CQ, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com
        Attention needed from Jianlin Qiu

        Dale Curtis added 1 comment

        Patchset-level comments
        File-level comment, Patchset 19:
        Dale Curtis . resolved

        Jianlin, can you pare down to just the required reviewers?

        Dale Curtis

        Acknowledged

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Jianlin Qiu
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Review
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
        Gerrit-Change-Number: 5456050
        Gerrit-PatchSet: 20
        Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
        Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
        Gerrit-Reviewer: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Reviewer: Sergey Silkin <ssi...@chromium.org>
        Gerrit-CC: Erik Språng <spr...@chromium.org>
        Gerrit-CC: Harald Alvestrand <h...@chromium.org>
        Gerrit-CC: Hirokazu Honda <hi...@chromium.org>
        Gerrit-CC: Ilya Nikolaevskiy <il...@chromium.org>
        Gerrit-CC: Philip Eliasson <phil...@webrtc.org>
        Gerrit-Attention: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Comment-Date: Wed, 24 Apr 2024 19:48:51 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Dale Curtis <dalec...@chromium.org>
        satisfied_requirement
        open
        diffy

        Jianlin Qiu (Gerrit)

        unread,
        Apr 24, 2024, 5:18:16 PMApr 24
        to Dale Curtis, Mirko Bonadei, Daniel Cheng, Sergey Silkin, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Chromium LUCI CQ, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com

        Jianlin Qiu voted and added 1 comment

        Votes added by Jianlin Qiu

        Commit-Queue+2

        1 comment

        Patchset-level comments
        Jianlin Qiu . resolved

        Thanks all for the review! Submitting.

        Open in Gerrit

        Related details

        Attention set is empty
        Gerrit-Comment-Date: Wed, 24 Apr 2024 21:17:56 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        open
        diffy

        Chromium LUCI CQ (Gerrit)

        unread,
        Apr 24, 2024, 6:22:52 PMApr 24
        to Jianlin Qiu, Dale Curtis, Mirko Bonadei, Daniel Cheng, Sergey Silkin, Ilya Nikolaevskiy, Harald Alvestrand, Philip Eliasson, Erik Språng, Hirokazu Honda, Tricium, chromium...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, poscia...@chromium.org, video-networking...@google.com

        Chromium LUCI CQ submitted the change

        Change information

        Commit message:
        RTC encoder/decoder factory support for H.265 behind flag.

        This allows H.265 to be negotiated when build with RTC H.265 support, and at the same time necessary runtime flags are turned on for H.265.

        A few flags needs to be provided at runtime for H.265 streaming:
        First, to allow H.265 to be included in sender's capabilities,
        --enable-features=PlatformHEVCEncoderSupport should be specified so that
        encoder GPU factory will enumerate HW H.265 encoders;
        --enable-features=WebRtcAllowH265Send should be specified so that RTC
        encoder factory allows H.265 to be included in supported formats when GPU encoder factory reports supporting H.265.
        --enable-features=WebRtcAllowH265Receive should be specified so that RTC
        decoder factory allows H.265 to be included in supported formats when GPU decoder factory reports supporting H.265.
        --force-fieldtrials=WebRTC-Video-H26xPacketBuffer/Enabled needs to be specified to switch frame assembly implementation to H.26x specific instead of using generic packet buffer.
        Bug: webrtc:13485
        Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
        Reviewed-by: Mirko Bonadei <mbon...@chromium.org>
        Commit-Queue: Jianlin Qiu <jianl...@intel.com>
        Reviewed-by: Sergey Silkin <ssi...@chromium.org>
        Reviewed-by: Daniel Cheng <dch...@chromium.org>
        Reviewed-by: Dale Curtis <dalec...@chromium.org>
        Cr-Commit-Position: refs/heads/main@{#1292129}
        Files:
        • M media/webrtc/webrtc_features.cc
        • M media/webrtc/webrtc_features.h
        • M third_party/blink/renderer/platform/BUILD.gn
        • M third_party/blink/renderer/platform/peerconnection/DEPS
        • M third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc
        • M third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter_test.cc
        • M third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory.cc
        • M third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_factory_test.cc
        • M third_party/blink/renderer/platform/peerconnection/rtc_video_encoder.cc
        • M third_party/blink/renderer/platform/peerconnection/rtc_video_encoder_factory.cc
        • M third_party/blink/renderer/platform/peerconnection/rtc_video_encoder_factory_test.cc
        • M third_party/blink/renderer/platform/webrtc/webrtc_video_utils.cc
        • M third_party/webrtc_overrides/BUILD.gn
        Change size: L
        Delta: 13 files changed, 416 insertions(+), 29 deletions(-)
        Branch: refs/heads/main
        Submit Requirements:
        • requirement satisfiedCode-Review: +1 by Dale Curtis, +1 by Daniel Cheng, +1 by Sergey Silkin, +1 by Mirko Bonadei
        Open in Gerrit
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: merged
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Ifb2a3e3b7f09fa1c2307629e08d855a8ece53531
        Gerrit-Change-Number: 5456050
        Gerrit-PatchSet: 21
        Gerrit-Owner: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
        Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
        Gerrit-Reviewer: Daniel Cheng <dch...@chromium.org>
        Gerrit-Reviewer: Jianlin Qiu <jianl...@intel.com>
        Gerrit-Reviewer: Mirko Bonadei <mbon...@chromium.org>
        Gerrit-Reviewer: Sergey Silkin <ssi...@chromium.org>
        open
        diffy
        satisfied_requirement
        Reply all
        Reply to author
        Forward
        0 new messages