New issue
Advanced search Search tips

Issue 766728 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

[Desktop Capture] Update DesktopMediaID to handle audio/video capture

Project Member Reported by mfo...@chromium.org, Sep 19 2017

Issue description

From https://chromium-review.googlesource.com/c/chromium/src/+/656597/6/content/public/browser/desktop_media_id.cc#176


 I'm suggesting to replace "audio_share" boolean field in DesktopMediaID with an enum field that will indicated what type of media is being requested. It would have have 3 allowed values: AUDIO, VIDEO and AUDIO_VIDEO (probably better to make it a bitmask with two bits for audio and video). Type enum will stay the same.

Currently 'type' indicates the source type, while 'audio_share' flag indicates type of the media that's being received from that source (video or video+audio). So |type| is not the best name for that field. It may be better if we rename it to source_type. 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 27 2017

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

commit 9764561c373b3afa9eb6ec9bcc181eb72c23e1a3
Author: mark a. foltz <mfoltz@chromium.org>
Date: Wed Sep 27 05:55:21 2017

[Desktop Capture] Replace audio_share with an enum.

This is a followup change to
https://chromium-review.googlesource.com/c/chromium/src/+/656597

Changes in DesktopMediaId:
- Adds a Capture enum to indicate what is being captured
- Type enum renamed to Source to distinguish it from Capture
- type renamed to source_type
- audio_share boolean replaced with capture_type enum
- Adds methods to query/set capture type

Bug:  766728 
Change-Id: I95c6c8ec4b12a7c2e78b360843f91956b223dcfa
Reviewed-on: https://chromium-review.googlesource.com/673093
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Zijie He <zijiehe@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504590}
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.h
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/desktop_media_list.h
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/desktop_media_list_ash.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/desktop_media_list_ash.h
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/desktop_media_list_ash_unittest.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/desktop_media_list_base.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/desktop_media_list_base.h
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/fake_desktop_media_list.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/fake_desktop_media_list.h
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/native_desktop_media_list.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/native_desktop_media_list.h
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/native_desktop_media_list_unittest.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/screen_capture_infobar_delegate_android.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/tab_desktop_media_list.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/window_icon_util_chromeos.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/window_icon_util_mac.mm
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/window_icon_util_ozone.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/window_icon_util_win.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/media/webrtc/window_icon_util_x11.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/ui/cocoa/media_picker/desktop_media_picker_controller.mm
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/ui/cocoa/media_picker/desktop_media_picker_controller_unittest.mm
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.h
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_browsertest.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/browser/media/capture/desktop_capture_device.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/browser/media/capture/desktop_capture_device.h
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/browser/media/capture/desktop_capture_device_aura.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/browser/media/capture/desktop_capture_device_aura_unittest.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/browser/media/capture/desktop_capture_device_unittest.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/public/browser/desktop_media_id.cc
[modify] https://crrev.com/9764561c373b3afa9eb6ec9bcc181eb72c23e1a3/content/public/browser/desktop_media_id.h

Comment 2 by mfo...@chromium.org, Sep 27 2017

Status: Fixed (was: Assigned)
DesktopMediaID could also use a unit test.  I'll leave that as an exercise for the next person to touch it (possibly me :)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 29 2017

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

commit 608462600d066c654472da7859c4290698a07b61
Author: mark a. foltz <mfoltz@chromium.org>
Date: Fri Sep 29 05:43:37 2017

Revert "[Desktop Capture] Replace audio_share with an enum."

This reverts commit 9764561c373b3afa9eb6ec9bcc181eb72c23e1a3.

Reason for revert: Need to revert https://chromium-review.googlesource.com/c/chromium/src/+/656597, which this patch depends on.

Original change's description:
> [Desktop Capture] Replace audio_share with an enum.
> 
> This is a followup change to
> https://chromium-review.googlesource.com/c/chromium/src/+/656597
> 
> Changes in DesktopMediaId:
> - Adds a Capture enum to indicate what is being captured
> - Type enum renamed to Source to distinguish it from Capture
> - type renamed to source_type
> - audio_share boolean replaced with capture_type enum
> - Adds methods to query/set capture type
> 
> Bug:  766728 
> Change-Id: I95c6c8ec4b12a7c2e78b360843f91956b223dcfa
> Reviewed-on: https://chromium-review.googlesource.com/673093
> Commit-Queue: mark a. foltz <mfoltz@chromium.org>
> Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
> Reviewed-by: Zijie He <zijiehe@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#504590}

TBR=sky@chromium.org,mfoltz@chromium.org,sergeyu@chromium.org,guidou@chromium.org,jochen@chromium.org,zijiehe@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  766728 
Change-Id: I844800849ec527c12ee7330b45bc90a25f12dc9e
Reviewed-on: https://chromium-review.googlesource.com/691107
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505307}
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/extensions/api/desktop_capture/desktop_capture_apitest.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.h
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/desktop_capture_access_handler.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/desktop_media_list.h
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/desktop_media_list_ash.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/desktop_media_list_ash.h
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/desktop_media_list_ash_unittest.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/desktop_media_list_base.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/desktop_media_list_base.h
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/fake_desktop_media_list.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/fake_desktop_media_list.h
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/native_desktop_media_list.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/native_desktop_media_list.h
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/native_desktop_media_list_unittest.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/screen_capture_infobar_delegate_android.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/tab_desktop_media_list.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/window_icon_util_chromeos.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/window_icon_util_mac.mm
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/window_icon_util_ozone.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/window_icon_util_win.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/media/webrtc/window_icon_util_x11.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/ui/cocoa/media_picker/desktop_media_picker_controller.mm
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/ui/cocoa/media_picker/desktop_media_picker_controller_unittest.mm
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/ui/views/desktop_capture/desktop_media_list_view.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.h
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_browsertest.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/browser/media/capture/desktop_capture_device.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/browser/media/capture/desktop_capture_device.h
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/browser/media/capture/desktop_capture_device_aura.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/browser/media/capture/desktop_capture_device_aura_unittest.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/browser/media/capture/desktop_capture_device_unittest.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/public/browser/desktop_media_id.cc
[modify] https://crrev.com/608462600d066c654472da7859c4290698a07b61/content/public/browser/desktop_media_id.h

Sign in to add a comment