[fuchsia] Move VideoFrameWriterQueue to separate file [chromium/src : main]

0 views
Skip to first unread message

Zijie He (Gerrit)

unread,
Jun 26, 2026, 3:26:28 PM (13 days ago) Jun 26
to David Song, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, fuchsia...@chromium.org
Attention needed from David Song

Zijie He added 1 comment

File media/fuchsia/video/BUILD.gn
Line 53, Patchset 31: "video_frame_writer_queue_unittest.cc",
David Song . resolved

I think we should either:

1. move VideoFrameWriterQueue to a separate file
2. create fuchsia_video_encode_accelerator_unittest, which can test VideoFrameWriterQueue.

Zijie He

Ah, good idea, that's even cleaner.

Open in Gerrit

Related details

Attention is currently required from:
  • David Song
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
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: Ia2605cd05836a308174b15e238b2ab87844ef4ce
Gerrit-Change-Number: 7900161
Gerrit-PatchSet: 35
Gerrit-Owner: Zijie He <zij...@google.com>
Gerrit-Reviewer: David Song <winter...@google.com>
Gerrit-Reviewer: Zijie He <zij...@google.com>
Gerrit-Attention: David Song <winter...@google.com>
Gerrit-Comment-Date: Fri, 26 Jun 2026 19:26:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: David Song <winter...@google.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

David Song (Gerrit)

unread,
Jul 8, 2026, 4:34:17 PM (21 hours ago) Jul 8
to Zijie He, Code Review Nudger, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, fuchsia...@chromium.org
Attention needed from Zijie He

David Song voted and added 2 comments

Votes added by David Song

Code-Review+1

2 comments

Patchset-level comments
File-level comment, Patchset 35 (Latest):
David Song . resolved

Thanks!

File media/fuchsia/video/video_frame_writer_queue_unittest.cc
Line 86, Patchset 35 (Latest): EXPECT_TRUE(base::test::RunUntil([&]() { return processed; }));
David Song . unresolved

Do we need `RunUntil` in this test? AFAICT `Enqueue` should start processing synchronously, so we should be ok with just a direct assertion?

Open in Gerrit

Related details

Attention is currently required from:
  • Zijie He
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    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: Ia2605cd05836a308174b15e238b2ab87844ef4ce
    Gerrit-Change-Number: 7900161
    Gerrit-PatchSet: 35
    Gerrit-Owner: Zijie He <zij...@google.com>
    Gerrit-Reviewer: David Song <winter...@google.com>
    Gerrit-Reviewer: Zijie He <zij...@google.com>
    Gerrit-CC: Code Review Nudger <android-build...@prod.google.com>
    Gerrit-Attention: Zijie He <zij...@google.com>
    Gerrit-Comment-Date: Wed, 08 Jul 2026 20:34:03 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Zijie He (Gerrit)

    unread,
    12:04 PM (1 hour ago) 12:04 PM
    to David Song, Code Review Nudger, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, fuchsia...@chromium.org
    Attention needed from David Song

    Zijie He added 1 comment

    File media/fuchsia/video/video_frame_writer_queue_unittest.cc
    Line 86, Patchset 35 (Latest): EXPECT_TRUE(base::test::RunUntil([&]() { return processed; }));
    David Song . unresolved

    Do we need `RunUntil` in this test? AFAICT `Enqueue` should start processing synchronously, so we should be ok with just a direct assertion?

    Zijie He

    For this test case, very likely it would be handled immediately. But I think in general it's against the API definition and relying on the implementation details.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • David Song
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    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: Ia2605cd05836a308174b15e238b2ab87844ef4ce
    Gerrit-Change-Number: 7900161
    Gerrit-PatchSet: 35
    Gerrit-Owner: Zijie He <zij...@google.com>
    Gerrit-Reviewer: David Song <winter...@google.com>
    Gerrit-Reviewer: Zijie He <zij...@google.com>
    Gerrit-CC: Code Review Nudger <android-build...@prod.google.com>
    Gerrit-Attention: David Song <winter...@google.com>
    Gerrit-Comment-Date: Thu, 09 Jul 2026 16:04:42 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages