Add IsApplicationLoopbackCaptureSupported helper function. [chromium/src : main]
Olov Brändström (Gerrit)
Olov Brändström voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Henrik Andreasson
- Olga Sharonova
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Henrik Andreasson (Gerrit)
Henrik Andreasson added 1 comment![]( "Open in Gerrit")
Is it correct that we do the same today for Windows as is stated in the title?
On Windows, this function checks if the OS is Windows 11 or newer,
aligning the logic with `IsRestrictOwnAudioSupported()`.
Related details
- Olga Sharonova
- Olov Brändström
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olov Brändström (Gerrit)
Olov Brändström added 1 comment![]( "Open in Gerrit")
Is it correct that we do the same today for Windows as is stated in the title?
On Windows, this function checks if the OS is Windows 11 or newer,
aligning the logic with `IsRestrictOwnAudioSupported()`.
- Henrik Andreasson
- Olga Sharonova
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Henrik Andreasson (Gerrit)
Henrik Andreasson voted Code-Review+1![]( "Open in Gerrit")
- Olga Sharonova
- Olov Brändström
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olga Sharonova (Gerrit)
Olga Sharonova added 1 comment![]( "Open in Gerrit")
bool IsApplicationAudioCaptureSupported() {Can we move everything to media_switches? It does not seem there are any benefits in splitting logic between two places
Related details
- Olov Brändström
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. |
Olov Brändström (Gerrit)
Olov Brändström added 1 comment![]( "Open in Gerrit")
Can we move everything to media_switches? It does not seem there are any benefits in splitting logic between two places
I've moved the feature flags to media_switches, to be able to move all logic there.
Related details
- Henrik Andreasson
- Olga Sharonova
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olov Brändström (Gerrit)
Olov Brändström added 1 comment![]( "Open in Gerrit")
Hi @to...@chromium.org. I added you as a reviewer since I need an owner of desktop_media_picker_views.
Related details
- Henrik Andreasson
- Olga Sharonova
- Tove Petersson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tove Petersson (Gerrit)
Tove Petersson added 2 comments![]( "Open in Gerrit")
base::win::GetVersion() >= base::win::Version::WIN11;Should this really be checked as part of the feature test.
When cleaning up these feature flags, I assume that this test will be done in the implementation files. Can it be done there already? Or is it part of the rollout to start on Win11 and later expand to other Windows versions?
base::win::GetVersion() >= base::win::Version::WIN11;Can you add a comment about why it is disabled on Windows 10? From what I gather, application audio is supported on at least some versions of Windows 10?
Related details
- Henrik Andreasson
- Olga Sharonova
- Olov Brändström
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. |
Olov Brändström (Gerrit)
Olov Brändström added 2 comments![]( "Open in Gerrit")
base::win::GetVersion() >= base::win::Version::WIN11;Should this really be checked as part of the feature test.
When cleaning up these feature flags, I assume that this test will be done in the implementation files. Can it be done there already? Or is it part of the rollout to start on Win11 and later expand to other Windows versions?
I followed the style of the existing functions, e.g. `IsMacCatapSystemLoopbackCaptureSupported()` and`IsWindowsProcessLoopbackCaptureSupported()`.
base::win::GetVersion() >= base::win::Version::WIN11;Can you add a comment about why it is disabled on Windows 10? From what I gather, application audio is supported on at least some versions of Windows 10?
This uses the same API as RestrictOwnAudio. `IsRestrictOwnAudioSupported()` checks for windows 11, so it should be the same for application audio capture. I'm not the expert, so if you want a better answer I could dig for it, but the reason we just enable for win11 is that we can't see if the user uses a win10 that support this API or not.
Related details
- Henrik Andreasson
- Olga Sharonova
- Tove Petersson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tove Petersson (Gerrit)
Tove Petersson voted Code-Review+1![]( "Open in Gerrit")
- Henrik Andreasson
- Olga Sharonova
- Olov Brändström
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olga Sharonova (Gerrit)
Olga Sharonova added 2 comments![]( "Open in Gerrit")
if (WindowAudioPreference() ==Why there is no update of this logic in the follow-up CL?
base::win::GetVersion() >= base::win::Version::WIN11;Olov BrändströmCan you add a comment about why it is disabled on Windows 10? From what I gather, application audio is supported on at least some versions of Windows 10?
This uses the same API as RestrictOwnAudio. `IsRestrictOwnAudioSupported()` checks for windows 11, so it should be the same for application audio capture. I'm not the expert, so if you want a better answer I could dig for it, but the reason we just enable for win11 is that we can't see if the user uses a win10 that support this API or not.
IsWindowsProcessLoopbackCaptureSupported() should be used instead.
Related details
- Henrik Andreasson
- Olov Brändström
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. |
Olov Brändström (Gerrit)
Olov Brändström added 2 comments![]( "Open in Gerrit")
Why there is no update of this logic in the follow-up CL?
The code reads: If audio should be offered in the window tab, the info box should say "application audio" if {windowAudio: window} and application audio loopback is supported, and "system audio" otherwise. That is true before and after any change.
The difference in the follow-up CL logic is just how often audio should be offered in the window tab, i.e. what `ShouldOfferWindowAudio()` returns.
base::win::GetVersion() >= base::win::Version::WIN11;Olov BrändströmCan you add a comment about why it is disabled on Windows 10? From what I gather, application audio is supported on at least some versions of Windows 10?
Olga SharonovaThis uses the same API as RestrictOwnAudio. `IsRestrictOwnAudioSupported()` checks for windows 11, so it should be the same for application audio capture. I'm not the expert, so if you want a better answer I could dig for it, but the reason we just enable for win11 is that we can't see if the user uses a win10 that support this API or not.
IsWindowsProcessLoopbackCaptureSupported() should be used instead.
- Henrik Andreasson
- Olga Sharonova
- Tove Petersson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |