New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 793255 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Update AudioParameter fields from content::MediaStreamDevice

Project Member Reported by guidou@chromium.org, Dec 8 2017

Issue description

The |input| filed should be base::Optional instead of using UnvailableDeviceParams() to signal absence of parameters.

This |matched_output| field should be removed. It is currently used only to decide if remote audio tracks should be rendered to the associated output device. If the values are valid (e.g., sample_rate > 0 and so on) rendering to associated sink is allowed. However, the parameters that are actually used are obtained from the output device to be used.
Possible alternatives are to use the |matched_output_device_id| field, and/or propagate the value of the renderToAssociatedSink constrainable property.
 
Components: Blink>GetUserMedia Blink>WebRTC>Audio
Owner: guidou@chromium.org
Status: Assigned (was: Untriaged)
Labels: -Pri-3 Pri-2
Cc: olka@chromium.org
Summary: Update AudioParameter fields from content::MediaStreamDevice (was: Remove |matched_output| field from content::MediaStreamDevice)
Description: Show this description
guidou@: I am somewhat familiar with this part of the code. If you have not begun working on this, can I take up this task? I myself was looking to cut down few fields from MediaStreamDevice as much as possible, to simplify typemapping for this Struct.
Cc: guidou@chromium.org
Owner: c.pa...@samsung.com
Yes, you can take it. 
guidou@: The |input| field, being a AudioParameter has to be valid for it to be sent over IPC/mojo. So, making it base::Optional instead of using UnvailableDeviceParams() be appropriate? How do we ensure that |input| is valid?
For |input|, the change is basically to replace UnvailableDeviceParams() with no value (nullopt). nullopt is serializable.

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c

commit 90ebce39c2a621d47c5e3c8d84bc140d01d2f67c
Author: Chandan Padhi <c.padhi@samsung.com>
Date: Fri Dec 15 03:56:44 2017

Remove |matched_output| field from content::MediaStreamDevice

Instead, |matched_output_device_id| will be used to fetch output audio parameters
for the associated output device or to decide if remote audio tracks should be
rendered to the associated output device.

Bug: 793255
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ic2954a6f6e99597c70b26abfb15a6c69d65b848a
Reviewed-on: https://chromium-review.googlesource.com/822311
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Chandan Padhi (OOO Fri) <c.padhi@samsung.com>
Cr-Commit-Position: refs/heads/master@{#524315}
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/browser/renderer_host/media/audio_input_device_manager.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/browser/renderer_host/media/audio_input_device_manager.h
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/browser/renderer_host/media/audio_output_authorization_handler.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/browser/renderer_host/media/audio_output_authorization_handler.h
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/common/media/media_stream_param_traits.h
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/public/common/media_stream_request.h
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/renderer/media/media_stream_renderer_factory_impl.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/renderer/media/mock_mojo_media_stream_dispatcher_host.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/renderer/media/user_media_client_impl_unittest.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/renderer/media/user_media_processor.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/renderer/media/webrtc/processed_local_audio_source.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/renderer/media/webrtc_audio_device_impl.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/content/renderer/media/webrtc_audio_device_impl.h
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/media/audio/audio_system.h
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/media/audio/audio_system_helper.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/media/audio/audio_system_test_util.cc
[modify] https://crrev.com/90ebce39c2a621d47c5e3c8d84bc140d01d2f67c/media/audio/audio_system_test_util.h

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a14d2f8335c7c0d888057cb24bbcefcbcf068081

commit a14d2f8335c7c0d888057cb24bbcefcbcf068081
Author: Chandan Padhi <c.padhi@samsung.com>
Date: Wed Dec 20 16:53:03 2017

Update MediaStreamDevice's |matched_output_device_id| to use base::Optional

|matched_output_device_id| as an empty string doesn't necessarily mean the
absence of an output device. It can also represent a default output device.
Changing |matched_output_device_id| to base::Optional will allow us to use
base::nullopt to indicate the absence of an output device.

Bug: 793255
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If405534df9c8b558a56a1a862f6aa72ce34df5dc
Reviewed-on: https://chromium-review.googlesource.com/833828
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Chandan Padhi <c.padhi@samsung.com>
Cr-Commit-Position: refs/heads/master@{#525350}
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.h
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/content/browser/renderer_host/media/audio_input_device_manager.cc
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/content/browser/renderer_host/media/audio_input_device_manager.h
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/content/browser/renderer_host/media/audio_output_authorization_handler.cc
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/content/public/common/media_stream_request.h
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/content/renderer/media/user_media_client_impl_unittest.cc
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/content/renderer/media/user_media_processor.cc
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/content/renderer/media/webrtc_audio_device_impl.cc
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/media/audio/audio_system.h
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/media/audio/audio_system_helper.cc
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/media/audio/audio_system_test_util.cc
[modify] https://crrev.com/a14d2f8335c7c0d888057cb24bbcefcbcf068081/media/audio/audio_system_test_util.h

Sign in to add a comment