Extracting VoiceIsolationHandler out of AudioProcessorHandler [chromium/src : main]

0 views
Skip to first unread message

Pablo Barrera González (Gerrit)

unread,
5:41 AM (12 hours ago) 5:41 AM
to chromium...@chromium.org, feature-me...@chromium.org, chrome-intell...@chromium.org, marinacio...@chromium.org, olka+...@chromium.org, chrome-intelligence-te...@google.com

Pablo Barrera González has uploaded the change for review

Commit message

Extracting VoiceIsolationHandler out of AudioProcessorHandler

The creation of VoiceIsolation should be handled in one place only. This change
consolidates the creation in the InputController as a previous step to move
everything inside VoiceIsolationHandler.
Bug: 512016773
Change-Id: I206fcbd3310acc330af60c806f07ae7c6c58df12

Change diff


Change information

Files:
  • M services/audio/audio_processor_handler.cc
  • M services/audio/audio_processor_handler.h
  • M services/audio/audio_processor_handler_unittest.cc
  • M services/audio/input_controller.cc
Change size: M
Delta: 4 files changed, 45 insertions(+), 31 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not 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: newchange
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I206fcbd3310acc330af60c806f07ae7c6c58df12
Gerrit-Change-Number: 8064707
Gerrit-PatchSet: 1
Gerrit-Owner: Pablo Barrera González <barr...@google.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

Olga Sharonova (Gerrit)

unread,
8:59 AM (9 hours ago) 8:59 AM
to Pablo Barrera González, Ale Bzk, Per Åhgren, Sam Zackrisson, chromium...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, feature-me...@chromium.org, marinacio...@chromium.org, olka+...@chromium.org
Attention needed from Pablo Barrera González

Olga Sharonova added 1 comment

Commit Message
Line 7, Patchset 2 (Latest):Extracting VoiceIsolationHandler out of AudioProcessorHandler


The creation of VoiceIsolation should be handled in one place only. This change
consolidates the creation in the InputController as a previous step to move
everything inside VoiceIsolationHandler.
Olga Sharonova . unresolved

Could you rephrase? I'm not sure I understand what is being said. (VoiceIsolation is created in one place)

Open in Gerrit

Related details

Attention is currently required from:
  • Pablo Barrera González
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • 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: I206fcbd3310acc330af60c806f07ae7c6c58df12
    Gerrit-Change-Number: 8064707
    Gerrit-PatchSet: 2
    Gerrit-Owner: Pablo Barrera González <barr...@google.com>
    Gerrit-Reviewer: Olga Sharonova <ol...@chromium.org>
    Gerrit-CC: Ale Bzk <ales...@chromium.org>
    Gerrit-CC: Per Åhgren <pe...@chromium.org>
    Gerrit-CC: Sam Zackrisson <sa...@chromium.org>
    Gerrit-Attention: Pablo Barrera González <barr...@google.com>
    Gerrit-Comment-Date: Wed, 08 Jul 2026 12:59:35 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Olga Sharonova (Gerrit)

    unread,
    9:26 AM (8 hours ago) 9:26 AM
    to Pablo Barrera González, Ale Bzk, Per Åhgren, Sam Zackrisson, chromium...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, feature-me...@chromium.org, marinacio...@chromium.org, olka+...@chromium.org
    Attention needed from Pablo Barrera González

    Olga Sharonova added 2 comments

    File services/audio/input_controller.cc
    Line 435, Patchset 2 (Latest): return false;
    Olga Sharonova . unresolved

    I believe this will change when the base CL changes. It' undesirable that we do one thing here and another at l.448

    Line 466, Patchset 2 (Latest): voice_isolation_handler
    Olga Sharonova . unresolved

    Maybe it would be more readable and consistent if we always pass it but the constructor ignores it if voice_isolation_handler is present?

    Gerrit-Comment-Date: Wed, 08 Jul 2026 13:26:19 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Pablo Barrera González (Gerrit)

    unread,
    11:14 AM (7 hours ago) 11:14 AM
    to Ale Bzk, Per Åhgren, Sam Zackrisson, Olga Sharonova, chromium...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, feature-me...@chromium.org, marinacio...@chromium.org, olka+...@chromium.org
    Attention needed from Olga Sharonova

    Pablo Barrera González added 1 comment

    File services/audio/input_controller.cc
    Line 466, Patchset 2 (Latest): voice_isolation_handler
    Olga Sharonova . unresolved

    Maybe it would be more readable and consistent if we always pass it but the constructor ignores it if voice_isolation_handler is present?

    Pablo Barrera González

    You mean passing the empty unique_ptr in deliver_process_audio_callback, right?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Olga Sharonova
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • 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: I206fcbd3310acc330af60c806f07ae7c6c58df12
    Gerrit-Change-Number: 8064707
    Gerrit-PatchSet: 2
    Gerrit-Owner: Pablo Barrera González <barr...@google.com>
    Gerrit-Reviewer: Olga Sharonova <ol...@chromium.org>
    Gerrit-CC: Ale Bzk <ales...@chromium.org>
    Gerrit-CC: Per Åhgren <pe...@chromium.org>
    Gerrit-CC: Sam Zackrisson <sa...@chromium.org>
    Gerrit-Attention: Olga Sharonova <ol...@chromium.org>
    Gerrit-Comment-Date: Wed, 08 Jul 2026 15:14:20 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Olga Sharonova <ol...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Olga Sharonova (Gerrit)

    unread,
    11:19 AM (6 hours ago) 11:19 AM
    to Pablo Barrera González, Ale Bzk, Per Åhgren, Sam Zackrisson, Olga Sharonova, chromium...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, feature-me...@chromium.org, marinacio...@chromium.org, olka+...@chromium.org
    Attention needed from Olga Sharonova and Pablo Barrera González

    Olga Sharonova added 1 comment

    File services/audio/input_controller.cc
    Line 466, Patchset 2 (Latest): voice_isolation_handler
    Olga Sharonova . unresolved

    Maybe it would be more readable and consistent if we always pass it but the constructor ignores it if voice_isolation_handler is present?

    Pablo Barrera González

    You mean passing the empty unique_ptr in deliver_process_audio_callback, right?

    Olga Sharonova

    I don't understand. I mean always passing deliver_processed_audio_callback, and then discarding it in the constructor if voice_isolation_handler is no null.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Olga Sharonova
    • Pablo Barrera González
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • 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: I206fcbd3310acc330af60c806f07ae7c6c58df12
    Gerrit-Change-Number: 8064707
    Gerrit-PatchSet: 2
    Gerrit-Owner: Pablo Barrera González <barr...@google.com>
    Gerrit-Reviewer: Olga Sharonova <ol...@chromium.org>
    Gerrit-CC: Ale Bzk <ales...@chromium.org>
    Gerrit-CC: Olga Sharonova <ol...@google.com>
    Gerrit-CC: Per Åhgren <pe...@chromium.org>
    Gerrit-CC: Sam Zackrisson <sa...@chromium.org>
    Gerrit-Attention: Olga Sharonova <ol...@chromium.org>
    Gerrit-Attention: Pablo Barrera González <barr...@google.com>
    Gerrit-Comment-Date: Wed, 08 Jul 2026 15:19:32 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Olga Sharonova <ol...@chromium.org>
    Comment-In-Reply-To: Pablo Barrera González <barr...@google.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Pablo Barrera González (Gerrit)

    unread,
    4:05 PM (2 hours ago) 4:05 PM
    to Olga Sharonova, Ale Bzk, Per Åhgren, Sam Zackrisson, Olga Sharonova, chromium...@chromium.org, chrome-intell...@chromium.org, chrome-intelligence-te...@google.com, feature-me...@chromium.org, marinacio...@chromium.org, olka+...@chromium.org
    Attention needed from Olga Sharonova and Olga Sharonova

    Pablo Barrera González added 3 comments

    Commit Message
    Line 7, Patchset 2:Extracting VoiceIsolationHandler out of AudioProcessorHandler


    The creation of VoiceIsolation should be handled in one place only. This change
    consolidates the creation in the InputController as a previous step to move
    everything inside VoiceIsolationHandler.
    Olga Sharonova . resolved

    Could you rephrase? I'm not sure I understand what is being said. (VoiceIsolation is created in one place)

    Pablo Barrera González

    Done

    File services/audio/input_controller.cc
    Line 435, Patchset 2: return false;
    Olga Sharonova . resolved

    I believe this will change when the base CL changes. It' undesirable that we do one thing here and another at l.448

    Pablo Barrera González

    This is now part of MaybeCreateVoiceIsolationHandler.

    This is a refactor to move VoiceIsolation and VoiceIsolationHandler creation to the same place. In the follow up I will move VoiceIsolation creationg and model look up inside VoiceIsolationHandler so everything is handled in one place.

    Line 466, Patchset 2: voice_isolation_handler
    Olga Sharonova . resolved

    Maybe it would be more readable and consistent if we always pass it but the constructor ignores it if voice_isolation_handler is present?

    Pablo Barrera González

    You mean passing the empty unique_ptr in deliver_process_audio_callback, right?

    Olga Sharonova

    I don't understand. I mean always passing deliver_processed_audio_callback, and then discarding it in the constructor if voice_isolation_handler is no null.

    Pablo Barrera González

    I am now passing deliver_processed_audio_callback without weird tricks. Internally deliver_processed_audio_callback is used correctly checkig if voice_isolation_handler is valid before accessing it. I added a comment explaining this and I think it's better now.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Olga Sharonova
    • Olga Sharonova
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not 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: I206fcbd3310acc330af60c806f07ae7c6c58df12
      Gerrit-Change-Number: 8064707
      Gerrit-PatchSet: 5
      Gerrit-Owner: Pablo Barrera González <barr...@google.com>
      Gerrit-Reviewer: Olga Sharonova <ol...@chromium.org>
      Gerrit-CC: Ale Bzk <ales...@chromium.org>
      Gerrit-CC: Olga Sharonova <ol...@google.com>
      Gerrit-CC: Per Åhgren <pe...@chromium.org>
      Gerrit-CC: Sam Zackrisson <sa...@chromium.org>
      Gerrit-Attention: Olga Sharonova <ol...@chromium.org>
      Gerrit-Attention: Olga Sharonova <ol...@google.com>
      Gerrit-Comment-Date: Wed, 08 Jul 2026 20:05:08 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Olga Sharonova <ol...@chromium.org>
      Comment-In-Reply-To: Olga Sharonova <ol...@google.com>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages