RTCVDAdapter: Always delegate feature value in Vp9HWSupportForSL() [chromium/src : main]

0 views
Skip to first unread message

Hirokazu Honda (Gerrit)

unread,
Oct 25, 2021, 2:13:57 AM10/25/21
to Dale Curtis, Miguel Casas, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Fritz Koenig

Attention is currently required from: Dale Curtis, Miguel Casas.

Hirokazu Honda would like Dale Curtis and Miguel Casas to review this change.

View Change

RTCVDAdapter: Always delegate feature value in Vp9HWSupportForSL()

Originally RTCVideoDecoderAdapter::Vp9HWSupportForSpatialLayers()
returns media::kVaapiVp9kSVCHWDecoding feature value on ChromeOS x86
devices and otherwise false.

Some ChromeOS ARM devices, but not all of them, are capable of
decoding SVC streams. So we need to handle this within a platform
VideoDecoders for ChromeOS ARM; if a VideoDecoder detects a VP9 SVC
stream is not supported, then it triggers a software decoder
fallback.

This CL changes the Vp9HWSupportForSpatialLayers() so that it returns
media::kVaapiVp9kSVCHWDecoding on all platforms. It enables keeping
the feature disabled on non ChromeOS platform and relies ChromeOS
video decoders for the judgement. This also drops "Vaapi" from the
feature name.

Bug: b:203747982
Test: Meet on volteer and trogdor

Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
---
M third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc
M media/base/media_switches.cc
M media/gpu/test/video_player/video_player_test_environment.cc
M media/gpu/vp9_decoder.cc
M media/base/media_switches.h
M media/gpu/v4l2/v4l2_stateful_workaround.cc
6 files changed, 70 insertions(+), 28 deletions(-)


To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
Gerrit-Change-Number: 3230665
Gerrit-PatchSet: 4
Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
Gerrit-CC: Fritz Koenig <frko...@chromium.org>
Gerrit-Attention: Dale Curtis <dalec...@chromium.org>
Gerrit-Attention: Miguel Casas <mca...@chromium.org>
Gerrit-MessageType: newchange

Hirokazu Honda (Gerrit)

unread,
Oct 25, 2021, 2:14:01 AM10/25/21
to blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Miguel Casas, Dale Curtis, Fritz Koenig, chromium...@chromium.org

Attention is currently required from: Dale Curtis, Miguel Casas.

View Change

1 comment:

To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
Gerrit-Change-Number: 3230665
Gerrit-PatchSet: 4
Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
Gerrit-CC: Fritz Koenig <frko...@chromium.org>
Gerrit-Attention: Dale Curtis <dalec...@chromium.org>
Gerrit-Attention: Miguel Casas <mca...@chromium.org>
Gerrit-Comment-Date: Mon, 25 Oct 2021 06:13:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Dale Curtis (Gerrit)

unread,
Oct 25, 2021, 2:57:53 PM10/25/21
to Hirokazu Honda, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Miguel Casas, Fritz Koenig, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda, Miguel Casas.

Patch set 4:Code-Review +1

View Change

    To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
    Gerrit-Change-Number: 3230665
    Gerrit-PatchSet: 4
    Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
    Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
    Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
    Gerrit-CC: Fritz Koenig <frko...@chromium.org>
    Gerrit-CC: Tzung-Bi Shih <tzu...@chromium.org>
    Gerrit-Attention: Hirokazu Honda <hi...@chromium.org>
    Gerrit-Attention: Miguel Casas <mca...@chromium.org>
    Gerrit-Comment-Date: Mon, 25 Oct 2021 18:57:44 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Hirokazu Honda (Gerrit)

    unread,
    Oct 27, 2021, 2:10:24 AM10/27/21
    to blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Dale Curtis, Miguel Casas, Fritz Koenig, chromium...@chromium.org

    Attention is currently required from: Miguel Casas.

    View Change

    1 comment:

    To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
    Gerrit-Change-Number: 3230665
    Gerrit-PatchSet: 4
    Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
    Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
    Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
    Gerrit-CC: Fritz Koenig <frko...@chromium.org>
    Gerrit-CC: Tzung-Bi Shih <tzu...@chromium.org>
    Gerrit-Attention: Miguel Casas <mca...@chromium.org>
    Gerrit-Comment-Date: Wed, 27 Oct 2021 06:10:11 +0000

    Miguel Casas (Gerrit)

    unread,
    Oct 27, 2021, 2:33:06 PM10/27/21
    to Hirokazu Honda, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Dale Curtis, Fritz Koenig, chromium...@chromium.org

    Attention is currently required from: Hirokazu Honda.

    View Change

    4 comments:

    • Patchset:

      • Patch Set #4:

        Oh I forgot to send the comments. Basically the only
        Q is if it's OK to show the flag for all OSes, not
        only for CrOS.

    • File media/base/media_switches.cc:

      • Patch Set #4, Line 552: on ChromeOS.

        As the code stands, it'll be available to non-ChromeOS platforms,
        is that intended? Or should it be availble only on CrOS, enabled
        by default on X86 and disabled on ARM?

    • File media/gpu/test/video_player/video_player_test_environment.cc:

    • File media/gpu/vp9_decoder.cc:

    To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
    Gerrit-Change-Number: 3230665
    Gerrit-PatchSet: 4
    Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
    Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
    Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
    Gerrit-CC: Fritz Koenig <frko...@chromium.org>
    Gerrit-CC: Tzung-Bi Shih <tzu...@chromium.org>
    Gerrit-Attention: Hirokazu Honda <hi...@chromium.org>
    Gerrit-Comment-Date: Wed, 27 Oct 2021 18:32:53 +0000

    Hirokazu Honda (Gerrit)

    unread,
    Oct 28, 2021, 4:47:40 AM10/28/21
    to blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Dale Curtis, Miguel Casas, Fritz Koenig, chromium...@chromium.org

    Attention is currently required from: Miguel Casas.

    View Change

    3 comments:

    • File media/base/media_switches.cc:

      • As the code stands, it'll be available to non-ChromeOS platforms, […]

        Removed.

        Yes, it is intended to be visible on all OSs.
        I do so for future development and also to reduces if-defined.

    • File media/gpu/test/video_player/video_player_test_environment.cc:

      • Hmm, this is done by git-cl-format.

    • File media/gpu/vp9_decoder.cc:

      • Done

    To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
    Gerrit-Change-Number: 3230665
    Gerrit-PatchSet: 5
    Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
    Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
    Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
    Gerrit-CC: Fritz Koenig <frko...@chromium.org>
    Gerrit-CC: Tzung-Bi Shih <tzu...@chromium.org>
    Gerrit-Attention: Miguel Casas <mca...@chromium.org>
    Gerrit-Comment-Date: Thu, 28 Oct 2021 08:47:27 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Miguel Casas <mca...@chromium.org>
    Gerrit-MessageType: comment

    Miguel Casas (Gerrit)

    unread,
    Oct 28, 2021, 4:31:30 PM10/28/21
    to Hirokazu Honda, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Dale Curtis, Fritz Koenig, chromium...@chromium.org

    Attention is currently required from: Hirokazu Honda.

    Patch set 5:Code-Review +1

    View Change

      To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
      Gerrit-Change-Number: 3230665
      Gerrit-PatchSet: 5
      Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
      Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
      Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
      Gerrit-CC: Fritz Koenig <frko...@chromium.org>
      Gerrit-CC: Tzung-Bi Shih <tzu...@chromium.org>
      Gerrit-Attention: Hirokazu Honda <hi...@chromium.org>
      Gerrit-Comment-Date: Thu, 28 Oct 2021 20:31:22 +0000

      Hirokazu Honda (Gerrit)

      unread,
      Oct 28, 2021, 10:26:20 PM10/28/21
      to blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Miguel Casas, Dale Curtis, Fritz Koenig, chromium...@chromium.org

      Patch set 5:Commit-Queue +2

      View Change

        To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
        Gerrit-Change-Number: 3230665
        Gerrit-PatchSet: 5
        Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
        Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
        Gerrit-Reviewer: Hirokazu Honda <hi...@chromium.org>
        Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
        Gerrit-CC: Fritz Koenig <frko...@chromium.org>
        Gerrit-CC: Tzung-Bi Shih <tzu...@chromium.org>
        Gerrit-Comment-Date: Fri, 29 Oct 2021 02:26:01 +0000

        Chromium LUCI CQ (Gerrit)

        unread,
        Oct 28, 2021, 11:54:15 PM10/28/21
        to Hirokazu Honda, blink-...@chromium.org, feature-me...@chromium.org, kinuko...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Miguel Casas, Dale Curtis, Fritz Koenig, chromium...@chromium.org

        Chromium LUCI CQ submitted this change.

        View Change


        Approvals: Dale Curtis: Looks good to me Miguel Casas: Looks good to me Hirokazu Honda: Commit
        RTCVDAdapter: Always delegate feature value in Vp9HWSupportForSL()

        Originally RTCVideoDecoderAdapter::Vp9HWSupportForSpatialLayers()
        returns media::kVaapiVp9kSVCHWDecoding feature value on ChromeOS x86
        devices and otherwise false.

        Some ChromeOS ARM devices, but not all of them, are capable of
        decoding SVC streams. So we need to handle this within a platform
        VideoDecoders for ChromeOS ARM; if a VideoDecoder detects a VP9 SVC
        stream is not supported, then it triggers a software decoder
        fallback.

        This CL changes the Vp9HWSupportForSpatialLayers() so that it returns
        media::kVaapiVp9kSVCHWDecoding on all platforms. It enables keeping
        the feature disabled on non ChromeOS platform and relies ChromeOS
        video decoders for the judgement. This also drops "Vaapi" from the
        feature name.

        Bug: b:203747982
        Test: Meet on volteer and trogdor

        Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
        Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3230665
        Reviewed-by: Miguel Casas <mca...@chromium.org>
        Reviewed-by: Dale Curtis <dalec...@chromium.org>
        Commit-Queue: Hirokazu Honda <hi...@chromium.org>
        Cr-Commit-Position: refs/heads/main@{#936257}

        ---
        M third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc
        M media/base/media_switches.cc
        M media/gpu/test/video_player/video_player_test_environment.cc
        M media/gpu/vp9_decoder.cc
        M media/base/media_switches.h
        M media/gpu/v4l2/v4l2_stateful_workaround.cc
        6 files changed, 77 insertions(+), 30 deletions(-)


        To view, visit change 3230665. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I422264229043e27b73022eb9bd65e6ad688cc046
        Gerrit-Change-Number: 3230665
        Gerrit-PatchSet: 6
        Gerrit-Owner: Hirokazu Honda <hi...@chromium.org>
        Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
        Gerrit-Reviewer: Dale Curtis <dalec...@chromium.org>
        Gerrit-Reviewer: Hirokazu Honda <hi...@chromium.org>
        Gerrit-Reviewer: Miguel Casas <mca...@chromium.org>
        Gerrit-CC: Fritz Koenig <frko...@chromium.org>
        Gerrit-CC: Tzung-Bi Shih <tzu...@chromium.org>
        Gerrit-MessageType: merged
        Reply all
        Reply to author
        Forward
        0 new messages