Palak Agarwal (Gerrit)

unread,

Mar 4, 2026, 11:10:14 AM (yesterday) Mar 4

to Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova

Palak Agarwal voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Olga Sharonova (Gerrit)

unread,

Mar 4, 2026, 11:20:22 AM (yesterday) Mar 4

to Palak Agarwal, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Palak Agarwal

Olga Sharonova added 3 comments

File media/audio/mac/core_audio_util_mac.h

Line 81, Patchset 2 (Latest):

void SendLog(const LogCallback& log_callback,
             const char* func_name,
             const std::string& message,
             AudioObjectPropertySelector property_selector,
             AudioObjectID audio_object_id,
             OSStatus result = noErr);

Olga Sharonova . unresolved

Why is it public?

Line 72, Patchset 2 (Latest):std::optional<std::string> GetDeviceName(AudioObjectID device_id);

Olga Sharonova . unresolved

Why do we need them to be public?

File media/audio/mac/core_audio_util_mac.cc

Line 41, Patchset 2 (Latest):void SendLog(const LogCallback& log_callback,

Olga Sharonova . unresolved

If not public - place into an unnamed namespace?

Open in Gerrit

Related details

Attention is currently required from:

Palak Agarwal

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Olga Sharonova (Gerrit)

unread,

Mar 4, 2026, 11:21:32 AM (yesterday) Mar 4

to Palak Agarwal, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Palak Agarwal

Olga Sharonova added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Olga Sharonova . unresolved

Note: we don't need to do any drive-by improvements here, even though some stuff is not as neat as it should be.

Open in Gerrit

Related details

Attention is currently required from:

Palak Agarwal

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Palak Agarwal (Gerrit)

unread,

4:43 AM (10 hours ago) 4:43 AM

to Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova

Palak Agarwal added 3 comments

File media/audio/mac/core_audio_util_mac.h

Line 81, Patchset 2:void SendLog(const LogCallback& log_callback,


             const char* func_name,
             const std::string& message,
             AudioObjectPropertySelector property_selector,
             AudioObjectID audio_object_id,
             OSStatus result = noErr);

Olga Sharonova . resolved

Why is it public?

Palak Agarwal

You're right, this one doesn't need to be.

Line 72, Patchset 2:std::optional<std::string> GetDeviceName(AudioObjectID device_id);

Olga Sharonova . unresolved

Why do we need them to be public?

Palak Agarwal

It's in preparation for the next change where I'll have them as private members of the class so that it can use the log_callback_ member that we will introduce.

File media/audio/mac/core_audio_util_mac.cc

Line 41, Patchset 2:void SendLog(const LogCallback& log_callback,

Olga Sharonova . resolved

If not public - place into an unnamed namespace?

Palak Agarwal

Done

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Olga Sharonova (Gerrit)

unread,

8:00 AM (6 hours ago) 8:00 AM

to Palak Agarwal, Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova and Palak Agarwal

Olga Sharonova added 1 comment

File media/audio/mac/core_audio_util_mac.h

Line 72, Patchset 2:std::optional<std::string> GetDeviceName(AudioObjectID device_id);

Olga Sharonova . unresolved

Why do we need them to be public?

Palak Agarwal

It's in preparation for the next change where I'll have them as private members of the class so that it can use the log_callback_ member that we will introduce.

Olga Sharonova

I think I need to see this in context. To be clear: we cannot call these functions for the purpose of only getting the info to log. These are system calls, expensive and failure-prone.

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova
Palak Agarwal

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Palak Agarwal (Gerrit)

unread,

9:35 AM (5 hours ago) 9:35 AM

to Olga Sharonova, Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova and Olga Sharonova

Palak Agarwal added 1 comment

File media/audio/mac/core_audio_util_mac.h

Line 72, Patchset 2:std::optional<std::string> GetDeviceName(AudioObjectID device_id);

Olga Sharonova . resolved

Why do we need them to be public?

Palak Agarwal

It's in preparation for the next change where I'll have them as private members of the class so that it can use the log_callback_ member that we will introduce.

Olga Sharonova

I think I need to see this in context. To be clear: we cannot call these functions for the purpose of only getting the info to log. These are system calls, expensive and failure-prone.

Palak Agarwal

Okay, removing them. Also added the follow-up change that adds the enumeration callback

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova
Olga Sharonova

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Olga Sharonova (Gerrit)

unread,

10:04 AM (4 hours ago) 10:04 AM

to Palak Agarwal, Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova and Palak Agarwal

Olga Sharonova added 2 comments

File media/audio/mac/core_audio_util_mac.cc

Line 136, Patchset 4 (Parent):


std::optional<std::string> GetDeviceName(AudioObjectID device_id) {
  return GetDeviceStringProperty(device_id, kAudioObjectPropertyName);
}

std::optional<std::string> GetDeviceModel(AudioObjectID device_id) {
  return GetDeviceStringProperty(device_id, kAudioDevicePropertyModelUID);
}

Olga Sharonova . unresolved

Why do you keep doing this? I think they improve readability, and it's also just matter of taste, so if the authors/owners decided to have them - why to remove them?

Line 148, Patchset 4 (Latest): const char* func_name = __func__;

Olga Sharonova . unresolved

No need, ``__func__`` is already const char[]

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova
Palak Agarwal

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Palak Agarwal (Gerrit)

unread,

11:06 AM (3 hours ago) 11:06 AM

to Olga Sharonova, Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova and Olga Sharonova

Palak Agarwal added 2 comments

File media/audio/mac/core_audio_util_mac.cc

Line 136, Patchset 4 (Parent):


std::optional<std::string> GetDeviceName(AudioObjectID device_id) {
  return GetDeviceStringProperty(device_id, kAudioObjectPropertyName);
}

std::optional<std::string> GetDeviceModel(AudioObjectID device_id) {
  return GetDeviceStringProperty(device_id, kAudioDevicePropertyModelUID);
}

Olga Sharonova . resolved

Why do you keep doing this? I think they improve readability, and it's also just matter of taste, so if the authors/owners decided to have them - why to remove them?

Palak Agarwal

My thinking is - it just has a single usage and we will have to pass a log callback to it to get logs, making it more verbose. But if you want it, then we can keep it.

Line 148, Patchset 4: const char* func_name = __func__;

Olga Sharonova . resolved

No need, ``__func__`` is already const char[]

Palak Agarwal

Done

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova
Olga Sharonova

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Olga Sharonova (Gerrit)

unread,

11:10 AM (3 hours ago) 11:10 AM

to Palak Agarwal, Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova and Palak Agarwal

Olga Sharonova voted and added 1 comment

Votes added by Olga Sharonova

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Olga Sharonova . resolved

LGTM - thanks!

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova
Palak Agarwal

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Palak Agarwal (Gerrit)

unread,

11:33 AM (3 hours ago) 11:33 AM

to Olga Sharonova, Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova

Palak Agarwal added 1 comment

Patchset-level comments

File-level comment, Patchset 2:

Olga Sharonova . resolved

Note: we don't need to do any drive-by improvements here, even though some stuff is not as neat as it should be.

Palak Agarwal

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Palak Agarwal (Gerrit)

unread,

11:33 AM (3 hours ago) 11:33 AM

to Olga Sharonova, Olga Sharonova, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Attention needed from Olga Sharonova

Palak Agarwal voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Olga Sharonova

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

11:50 AM (2 hours ago) 11:50 AM

to Palak Agarwal, Olga Sharonova, Olga Sharonova, chromium...@chromium.org, feature-me...@chromium.org, mac-r...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Enhance Mac CoreAudio diagnostic logging

This change introduces a SendLog helper function to format and send log
messages (including OSStatus error descriptions) via the provided
LogCallback.

This CL will be followed up by a change that will pass the
AudioManager::LogCallback into CoreAudio utility functions to provide
better diagnostic information during audio device operations on macOS.

Bug: 484894482

Change-Id: Iefe93e7a46f10d6ecacd8558308699be7aafcc8d

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7633184

Commit-Queue: Palak Agarwal <agp...@chromium.org>

Reviewed-by: Olga Sharonova <ol...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1594725}

Files:

M media/audio/mac/core_audio_util_mac.cc
M media/audio/mac/core_audio_util_mac.h

Change size: M

Delta: 2 files changed, 126 insertions(+), 39 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Olga Sharonova

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Enhance Mac CoreAudio diagnostic logging [chromium/src : main]

Palak Agarwal (Gerrit)

Palak Agarwal voted Commit-Queue+1

Related details

Olga Sharonova (Gerrit)

Olga Sharonova added 3 comments

Related details

Olga Sharonova (Gerrit)

Olga Sharonova added 1 comment

Related details

Palak Agarwal (Gerrit)

Palak Agarwal added 3 comments

Related details

Olga Sharonova (Gerrit)

Olga Sharonova added 1 comment

Related details

Palak Agarwal (Gerrit)

Palak Agarwal added 1 comment

Related details

Olga Sharonova (Gerrit)

Olga Sharonova added 2 comments

Related details

Palak Agarwal (Gerrit)

Palak Agarwal added 2 comments

Related details

Olga Sharonova (Gerrit)

Olga Sharonova voted and added 1 comment

Votes added by Olga Sharonova

1 comment

Related details

Palak Agarwal (Gerrit)

Palak Agarwal added 1 comment

Related details

Palak Agarwal (Gerrit)

Palak Agarwal voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information