[camera_windows] Fixes initializing video preview with latest webcam driver #10303
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,8 @@ | |
| #include "string_utils.h" | ||
| #include "texture_handler.h" | ||
|
|
||
| enum class PlatformStreamCategory { video, photo, audio }; | ||
|
|
||
| namespace camera_windows { | ||
|
|
||
| using Microsoft::WRL::ComPtr; | ||
|
|
@@ -371,7 +373,8 @@ void CaptureControllerImpl::TakePicture(const std::string& file_path) { | |
| // Check MF_CAPTURE_ENGINE_PHOTO_TAKEN event handling | ||
| // for response process. | ||
| hr = photo_handler_->TakePhoto(file_path, capture_engine_.Get(), | ||
| base_capture_media_type_.Get(), | ||
| photo_source_stream_index_); | ||
| if (FAILED(hr)) { | ||
| // Destroy photo handler on error cases to make sure state is resetted. | ||
| photo_handler_ = nullptr; | ||
|
|
@@ -398,6 +401,41 @@ uint32_t CaptureControllerImpl::GetMaxPreviewHeight() const { | |
| } | ||
| } | ||
|
|
||
| // Gets the stream index for a given source, given a target stream category | ||
| HRESULT GetMediaSourceStreamIndex( | ||
| IMFCaptureSource* source, DWORD* source_stream_index, | ||
| PlatformStreamCategory target_stream_category) { | ||
| DWORD stream_count = 0; | ||
| HRESULT hr = source->GetDeviceStreamCount(&stream_count); | ||
| if (FAILED(hr)) { | ||
| return hr; | ||
| } | ||
|
|
||
| for (DWORD stream_index = 0; stream_index < stream_count; stream_index++) { | ||
| MF_CAPTURE_ENGINE_STREAM_CATEGORY stream_category; | ||
| hr = source->GetDeviceStreamCategory(stream_index, &stream_category); | ||
| if (FAILED(hr)) { | ||
| return hr; | ||
| } | ||
|
|
||
| if ((target_stream_category == PlatformStreamCategory::video && | ||
| (stream_category == MF_CAPTURE_ENGINE_STREAM_CATEGORY_VIDEO_PREVIEW || | ||
| stream_category == | ||
| MF_CAPTURE_ENGINE_STREAM_CATEGORY_VIDEO_CAPTURE)) || | ||
|
Collaborator
There was a problem hiding this comment. Why are preview and capture streams being treated interchangeably?
Contributor
Author
There was a problem hiding this comment. This is a similar behavior used in the Chromium source code (in VideoCaptureDeviceMFWin::FillCapabilities). Though I'm not too sure why they can be treated interchangeably.
Collaborator
There was a problem hiding this comment. That code is building a list of all relevant sources, it's not unilaterally picking a source to use. There is a huge difference between "a preview source is a potential video source" and "if the first source encountered is a preview source, always use it for video". It doesn't seem like the approach in this PR has been sufficiently thought through; I suggest writing up a short design document explaining your planned approach so that the approach can be discussed at a high level. |
||
| (target_stream_category == PlatformStreamCategory::photo && | ||
| (stream_category == | ||
| MF_CAPTURE_ENGINE_STREAM_CATEGORY_PHOTO_DEPENDENT || | ||
| stream_category == | ||
| MF_CAPTURE_ENGINE_STREAM_CATEGORY_PHOTO_INDEPENDENT)) || | ||
| (target_stream_category == PlatformStreamCategory::audio && | ||
| stream_category == MF_CAPTURE_ENGINE_STREAM_CATEGORY_AUDIO)) { | ||
| *source_stream_index = stream_index; | ||
| return S_OK; | ||
|
Collaborator
There was a problem hiding this comment. This is unconditionally returning only the first matching stream; why is that correct? How did you determine that unconditionally using the first stream is never a regression compared to using the preferred stream? That seems counterintuitive. |
||
| } | ||
| } | ||
| return E_FAIL; | ||
| } | ||
|
|
||
| // Finds best media type for given source stream index and max height; | ||
| bool FindBestMediaType(DWORD source_stream_index, IMFCaptureSource* source, | ||
| IMFMediaType** target_media_type, uint32_t max_height, | ||
|
|
@@ -472,23 +510,48 @@ HRESULT CaptureControllerImpl::FindBaseMediaTypes() { | |
|
|
||
| HRESULT CaptureControllerImpl::FindBaseMediaTypesForSource( | ||
| IMFCaptureSource* source) { | ||
| HRESULT hr; | ||
| hr = GetMediaSourceStreamIndex(source, &video_source_stream_index_, | ||
| PlatformStreamCategory::video); | ||
| if (FAILED(hr)) { | ||
| return E_FAIL; | ||
| } | ||
|
|
||
| hr = GetMediaSourceStreamIndex(source, &photo_source_stream_index_, | ||
| PlatformStreamCategory::photo); | ||
| if (FAILED(hr)) { | ||
| // Use the same source stream for photo as video on fail | ||
| photo_source_stream_index_ = video_source_stream_index_; | ||
| } | ||
|
|
||
| if (media_settings_.enable_audio()) { | ||
| hr = GetMediaSourceStreamIndex(source, &audio_source_stream_index_, | ||
| PlatformStreamCategory::audio); | ||
| if (FAILED(hr)) { | ||
| return E_FAIL; | ||
| } | ||
| } | ||
|
|
||
| // Find base media type for previewing. | ||
| if (!FindBestMediaType(video_source_stream_index_, source, | ||
| base_preview_media_type_.GetAddressOf(), | ||
| GetMaxPreviewHeight(), &preview_frame_width_, | ||
| &preview_frame_height_)) { | ||
| return E_FAIL; | ||
| } | ||
|
|
||
| hr = source->SetCurrentDeviceMediaType(video_source_stream_index_, | ||
| base_preview_media_type_.Get()); | ||
| if (FAILED(hr)) { | ||
| return E_FAIL; | ||
| } | ||
|
|
||
| // Find base media type for record and photo capture. | ||
| if (!FindBestMediaType(video_source_stream_index_, source, | ||
| base_capture_media_type_.GetAddressOf(), 0xffffffff, | ||
| nullptr, nullptr)) { | ||
| return E_FAIL; | ||
| } | ||
| return S_OK; | ||
| } | ||
|
|
||
|
|
@@ -523,8 +586,9 @@ void CaptureControllerImpl::StartRecord(const std::string& file_path) { | |
|
|
||
| // Check MF_CAPTURE_ENGINE_RECORD_STARTED event handling for response | ||
| // process. | ||
| hr = record_handler_->StartRecord( | ||
| file_path, capture_engine_.Get(), base_capture_media_type_.Get(), | ||
| video_source_stream_index_, audio_source_stream_index_); | ||
| if (FAILED(hr)) { | ||
| // Destroy record handler on error cases to make sure state is resetted. | ||
| record_handler_ = nullptr; | ||
|
|
@@ -610,9 +674,9 @@ void CaptureControllerImpl::StartPreview() { | |
|
|
||
| // Check MF_CAPTURE_ENGINE_PREVIEW_STARTED event handling for response | ||
| // process. | ||
| hr = preview_handler_->StartPreview( | ||
| capture_engine_.Get(), base_preview_media_type_.Get(), | ||
| video_source_stream_index_, capture_engine_callback_handler_.Get()); | ||
|
|
||
| if (FAILED(hr)) { | ||
| // Destroy preview handler on error cases to make sure state is resetted. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a declaration comment, per the style guide, explaining what it does and what its overall role is.