media/gpu: add AV1 for V4L2 stateless with chrome stack [chromium/src : main]

Steve Cho (Gerrit)

unread,

Aug 25, 2022, 4:06:43 PM8/25/22

to Hirokazu Honda, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org

Attention is currently required from: Hirokazu Honda.

Steve Cho would like Hirokazu Honda to review this change.

media/gpu: add AV1 for V4L2 stateless with chrome stack

This CL adds V4L2VideoDecoderDelegateAV1 class needed to support AV1
video decode for V4L2 stateless API with chrome stack.

Bug: b:243576271
TEST: autoninja -C out_cherry/Release media/gpu:video_decode_accelerator_tests
Change-Id: Iace448c9286abaa4ed96345b44fa1148f792f814
---
M media/gpu/av1_picture.h
M media/gpu/v4l2/BUILD.gn
A media/gpu/v4l2/v4l2_video_decoder_delegate_av1.cc
A media/gpu/v4l2/v4l2_video_decoder_delegate_av1.h
4 files changed, 105 insertions(+), 0 deletions(-)

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

Steve Cho (Gerrit)

unread,

Aug 25, 2022, 4:06:56 PM8/25/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

Patch set 1:Commit-Queue +1

View Change

1 comment:

Patchset:
- Patch Set #1:
  Hiro, PTAL. Thank you.

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

Steve Cho (Gerrit)

unread,

Aug 25, 2022, 4:22:20 PM8/25/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Chromium LUCI CQ, Hirokazu Honda, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

View Change

1 comment:

Patchset:
- Patch Set #1:
  Local build passed, but let me first follow up with dried run failed builds.
  Marked as "WIP".

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

Steve Cho (Gerrit)

unread,

Aug 25, 2022, 4:54:30 PM8/25/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Chromium LUCI CQ, Hirokazu Honda, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

Patch set 2:Commit-Queue +1

View Change

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

Steve Cho (Gerrit)

unread,

Aug 25, 2022, 7:14:21 PM8/25/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Chromium LUCI CQ, Hirokazu Honda, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

View Change

1 comment:

Patchset:
- Patch Set #2:
  win_optional_gpu_tests_rel failure seems irrelevant for me. running again.

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

Steve Cho (Gerrit)

unread,

Aug 25, 2022, 7:40:47 PM8/25/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Chromium LUCI CQ, Hirokazu Honda, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

View Change

1 comment:

Patchset:
- Patch Set #2:
  build failures seems fixed (most of them passing with few pending).
  Hiro, PTAL.

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

Hirokazu Honda (Gerrit)

unread,

Aug 25, 2022, 9:17:43 PM8/25/22

to Steve Cho, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Steve Cho.

Patch set 2:Code-Review +1

View Change

5 comments:

Commit Message:
- Patch Set #2, Line 10: video decode for V4L2 stateless API with chrome stack.
  Add clarification comment that the class is yet placeholder.
Patchset:
- Patch Set #2:
  LGTM % nits.
File media/gpu/av1_picture.h:
- Patch Set #2, Line 24: AsV4L2AV1Picture
  I think AsPicture() can be removed in favor of reinterpret_cast<>.
  But it's up to you to resolve the TODO before this CL.
File media/gpu/v4l2/BUILD.gn:
- Patch Set #2, Line 138:
```
  if (enable_libgav1_decoder || use_libgav1_parser) {
    deps += [ "//third_party/libgav1:libgav1" ]
  }
```
  I think building v4l2_video_decoder_delegate_av1 will require building libgav1.
  Furthermore, av1_decoder.h requires building libgav1.
  So I think we should assert(use_libgav1_parser) and just add libgav1 to deps.
  Note that enable_libgav1_decoder means to enable software libgav1 decoder.
  A hardware video encoder needs just its parser. So we should refer use_libgav1_parser.
  [1] https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/vaapi/BUILD.gn;l=36;drc=f3d9add239b921399f62b03cdede6373bcab5a72
File media/gpu/v4l2/v4l2_video_decoder_delegate_av1.h:
- Patch Set #2, Line 17: explicit
  explicit is unnecessary as there are two arguments.

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

Steve Cho (Gerrit)

unread,

Aug 26, 2022, 1:08:16 AM8/26/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, Chromium LUCI CQ, chromium...@chromium.org

View Change

1 comment:

File media/gpu/av1_picture.h:

- Patch Set #2, Line 24: AsV4L2AV1Picture

- I think AsPicture() can be removed in favor of reinterpret_cast<>. […]
  Thank you Hiro for the comment. I probably will leave TODO for this, as I see the same things for other files at the moment. I think it would be better to make changes for all files altogether.
  I think I did see that maybe something for VP9, which I didn't understand earlier. Now I see and understand what needs to be done with the casting for this.

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

Steve Cho (Gerrit)

unread,

Aug 26, 2022, 2:17:27 PM8/26/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

View Change

1 comment:

File media/gpu/v4l2/BUILD.gn:

Patch Set #2, Line 138:

  if (enable_libgav1_decoder || use_libgav1_parser) {
    deps += [ "//third_party/libgav1:libgav1" ]
  }

I think building v4l2_video_decoder_delegate_av1 will require building libgav1. […]

Thank you Hiro. Trying this. Let me think what needs to be done with the error.

ERROR at //media/gpu/v4l2/BUILD.gn:26:3: Assertion failed.
  assert(use_libgav1_parser)
  ^-----
See //BUILD.gn:1050:24: which caused the file to be included.
        data_deps += [ "//media/gpu/v4l2:v4l2_stateless_decoder" ]
                       ^----------------------------------------
FAILED: build.ninja.stamp 
../../buildtools/linux64/gn --root=../.. -q --regeneration gen .
ninja: error: rebuilding 'build.ninja': subcommand failed

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

Steve Cho (Gerrit)

unread,

Aug 26, 2022, 5:08:48 PM8/26/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

Patch set 2:Commit-Queue +1

View Change

3 comments:

Commit Message:

- Patch Set #2, Line 10: video decode for V4L2 stateless API with chrome stack.
  Add clarification comment that the class is yet placeholder.

- Done
File media/gpu/v4l2/BUILD.gn:

Patch Set #2, Line 138:

  if (enable_libgav1_decoder || use_libgav1_parser) {
    deps += [ "//third_party/libgav1:libgav1" ]
  }

- Thank you Hiro. Trying this. Let me think what needs to be done with the error. […]
  added more comments
  https://buganizer.corp.google.com/issues/243576271#comment22
  but added a comment for TODO.
  TL DR; `enable_libgav1_decoder` is actually used and `use_libgav1_parser` has to be updated.
File media/gpu/v4l2/v4l2_video_decoder_delegate_av1.h:

- Patch Set #2, Line 17: explicit
  explicit is unnecessary as there are two arguments.

- Done

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

Steve Cho (Gerrit)

unread,

Aug 26, 2022, 5:13:27 PM8/26/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

Patch set 4:Commit-Queue +1

View Change

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

Steve Cho (Gerrit)

unread,

Aug 26, 2022, 6:14:35 PM8/26/22

to feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Hirokazu Honda.

Patch set 4:Commit-Queue +2

View Change

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

Chromium LUCI CQ (Gerrit)

unread,

Aug 26, 2022, 6:18:50 PM8/26/22

to Steve Cho, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, chromium...@chromium.org

Chromium LUCI CQ submitted this change.

View Change



2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: media/gpu/v4l2/BUILD.gn
Insertions: 1, Deletions: 0.

@@ -135,6 +135,7 @@
     ]
   }
 
+  # TODO(b/243970152): update use_libgav1_parser flag


   if (enable_libgav1_decoder || use_libgav1_parser) {
     deps += [ "//third_party/libgav1:libgav1" ]
   }

```
```
The name of the file: media/gpu/v4l2/v4l2_video_decoder_delegate_av1.h
Insertions: 2, Deletions: 3.

@@ -14,9 +14,8 @@
 
 class V4L2VideoDecoderDelegateAV1 : public AV1Decoder::AV1Accelerator {
  public:
-  explicit V4L2VideoDecoderDelegateAV1(
-      V4L2DecodeSurfaceHandler* surface_handler,
-      V4L2Device* device);
+  V4L2VideoDecoderDelegateAV1(V4L2DecodeSurfaceHandler* surface_handler,
+                              V4L2Device* device);
 
   V4L2VideoDecoderDelegateAV1(const V4L2VideoDecoderDelegateAV1&) = delete;
   V4L2VideoDecoderDelegateAV1& operator=(const V4L2VideoDecoderDelegateAV1&) =
```
```
The name of the file: media/gpu/av1_picture.h
Insertions: 2, Deletions: 0.

@@ -21,6 +21,8 @@
   AV1Picture(const AV1Picture&) = delete;
   AV1Picture& operator=(const AV1Picture&) = delete;
 
+  // TODO(stevecho): remove to use reinterpret_cast<> instead
+  // when similar changes are made with other codecs (vp8, vp9, etc.)
   virtual V4L2AV1Picture* AsV4L2AV1Picture();
 
   // Create a duplicate instance and copy the data to it. It is used to support
```

Approvals:
  Hirokazu Honda: Looks good to me
  Steve Cho: Commit

media/gpu: add AV1 for V4L2 stateless with chrome stack

This CL adds V4L2VideoDecoderDelegateAV1 class needed to support AV1

video decode for V4L2 stateless API with chrome stack.

Bug: b:243576271
TEST: autoninja -C out_cherry/Release media/gpu:video_decode_accelerator_tests
Change-Id: Iace448c9286abaa4ed96345b44fa1148f792f814

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3857237
Reviewed-by: Hirokazu Honda <hi...@chromium.org>
Commit-Queue: Steve Cho <stev...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1039997}
---
M media/gpu/av1_picture.cc


M media/gpu/av1_picture.h
M media/gpu/v4l2/BUILD.gn
A media/gpu/v4l2/v4l2_video_decoder_delegate_av1.cc
A media/gpu/v4l2/v4l2_video_decoder_delegate_av1.h

5 files changed, 115 insertions(+), 0 deletions(-)

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

Hirokazu Honda (Gerrit)

unread,

Aug 28, 2022, 10:08:27 PM8/28/22

to Chromium LUCI CQ, Steve Cho, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, chromium...@chromium.org

View Change

1 comment:

File media/gpu/v4l2/BUILD.gn:

- Patch Set #2, Line 138:

- ```
  if (enable_libgav1_decoder || use_libgav1_parser) {
    deps += [ "//third_party/libgav1:libgav1" ]
  }
```
  added more comments […]
  Looks like you need to enable use_libgav1_parser in the case target_cpu="arm". https://source.chromium.org/chromium/chromium/src/+/main:third_party/libgav1/options.gni;l=16;bpv=1;bpt=0
  [History]
  I set enable_libgav1_decoder to 1 for arm devices because libgav1 decoder optimizes ARM target mainly and I supposed it could be more performant than dav1d decoder.
  Thereafter, we need to build libgav1 for parser only for VA-API video decoder and thus I introduce use_libgav1_parser. It is also used for windows hw decoder.
  I doubt use_libgav1_parser is not required for arm devices (until cherry).
  Actually, tracking the git log, "arm64" is added there to resolve gn error. https://crbug.com/1157842
  But it's actually time to add "arm" there.
  As you know, our chorme is currently built as "arm" while the device architecture is arm64.

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

Steve Cho (Gerrit)

unread,

Aug 29, 2022, 12:20:24 AM8/29/22

to Chromium LUCI CQ, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, chromium...@chromium.org

View Change

1 comment:

File media/gpu/v4l2/BUILD.gn:

Patch Set #2, Line 138:

  if (enable_libgav1_decoder || use_libgav1_parser) {
    deps += [ "//third_party/libgav1:libgav1" ]
  }

- Looks like you need to enable use_libgav1_parser in the case target_cpu="arm". https://source. […]
  Thank you Hiro for the history.

- "As you know, our chorme is currently built as "arm" while the device architecture is arm64."

- I was not aware of this. May I ask more context / history on this?

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

Hirokazu Honda (Gerrit)

unread,

Aug 29, 2022, 1:07:17 AM8/29/22

to Chromium LUCI CQ, Steve Cho, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, chromium...@chromium.org

View Change

1 comment:

File media/gpu/v4l2/BUILD.gn:

Patch Set #2, Line 138:

  if (enable_libgav1_decoder || use_libgav1_parser) {
    deps += [ "//third_party/libgav1:libgav1" ]
  }

- Thank you Hiro for the history. […]
  I don't know the context about this.
  go/cros-arm64 list a few reasons.
  We are trying to build 64-bit userland.
  In fact, boards whose suffix is 64 is built 64bit.

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

Steve Cho (Gerrit)

unread,

Aug 29, 2022, 11:54:20 AM8/29/22

to Chromium LUCI CQ, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, chromium...@chromium.org

View Change

1 comment:

File media/gpu/v4l2/BUILD.gn:

Patch Set #2, Line 138:

  if (enable_libgav1_decoder || use_libgav1_parser) {
    deps += [ "//third_party/libgav1:libgav1" ]
  }

- I don't know the context about this. […]
  Thank you Hiro for the info. Yes, I think we need to update use_libgav1_parser with
  "arm".

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

David Dorwin (Gerrit)

unread,

Aug 31, 2022, 3:48:27 PM8/31/22

to Chromium LUCI CQ, Steve Cho, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, Hirokazu Honda, chromium...@chromium.org

Attention is currently required from: Steve Cho.

View Change

1 comment:

File media/gpu/av1_picture.h:
- Patch Set #5, Line 26: virtual V4L2AV1Picture* AsV4L2AV1Picture();
  Drive-by: Should this be ifdef'd to Linux and Chrome OS?
  This file is also used on Windows: https://crsrc.org/c/third_party/libgav1/options.gni;drc=21f8887b265fb8ff218042ed65573364611a319d;l=13. (The Windows build would fail if the .cc was not returning nullptr.)

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

Steve Cho (Gerrit)

unread,

Aug 31, 2022, 3:57:07 PM8/31/22

to Chromium LUCI CQ, feature-me...@chromium.org, media-cro...@chromium.org, poscia...@chromium.org, David Dorwin, Hirokazu Honda, chromium...@chromium.org

View Change

1 comment:

File media/gpu/av1_picture.h:

- Patch Set #5, Line 26: virtual V4L2AV1Picture* AsV4L2AV1Picture();

- Drive-by: Should this be ifdef'd to Linux and Chrome OS? […]
  Thank you David for the comment.
  I realized later that this is not needed for AV1. So I am trying to remove this line in the below CL.
  https://chromium-review.googlesource.com/c/chromium/src/+/3862664

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

Reply all

Reply to author

Forward