Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(270)

Issue 541603002: Fix if-statement in win video capture that ignores fake DirectShow filters (Closed)

Created:
6 years, 3 months ago by magjed_chromium
Modified:
6 years, 3 months ago
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Fix if-statement in win video capture that ignores fake DirectShow filters The parentheses were wrong previously in this if-statement: if ((wcsstr(str_ptr, L"(VFW)") == NULL) && lstrlenW(str_ptr) < name_length || (!(LowerCaseEqualsASCII(str_ptr, str_ptr + name_length, kGoogleCameraAdapter)))) The condition lstrlenW(str_ptr) < name_length should be grouped with the condition LowerCaseEqualsASCII(...), not wcsstr(str_ptr, L"(VFW)") == NULL. The check lstrlenW(str_ptr) < name_length is redundant, so I removed it. Committed: https://crrev.com/dcad7f4830bb4ff1ec0e0f96b9d5fe78ce302cb1 Cr-Commit-Position: refs/heads/master@{#293530}

Patch Set 1 #

Total comments: 2

Patch Set 2 : mcasas@ comments #

Total comments: 3

Patch Set 3 : mcasas@s and tommi@s comments #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -36 lines) Patch
M media/video/capture/win/video_capture_device_factory_win.cc View 1 2 3 chunks +33 lines, -36 lines 3 comments Download

Messages

Total messages: 13 (3 generated)
magjed_chromium
PTAL
6 years, 3 months ago (2014-09-04 09:04:19 UTC) #2
mcasas
Overall not too bad. One suggestion.I think you need to rebase (bots update failed). https://codereview.chromium.org/541603002/diff/1/media/video/capture/win/video_capture_device_factory_win.cc ...
6 years, 3 months ago (2014-09-04 13:31:18 UTC) #3
magjed_chromium
https://codereview.chromium.org/541603002/diff/1/media/video/capture/win/video_capture_device_factory_win.cc File media/video/capture/win/video_capture_device_factory_win.cc (right): https://codereview.chromium.org/541603002/diff/1/media/video/capture/win/video_capture_device_factory_win.cc#newcode143 media/video/capture/win/video_capture_device_factory_win.cc:143: if (SUCCEEDED(hr) && name.type() == VT_BSTR) { On 2014/09/04 ...
6 years, 3 months ago (2014-09-04 15:31:28 UTC) #4
mcasas
https://codereview.chromium.org/541603002/diff/20001/media/video/capture/win/video_capture_device_factory_win.cc File media/video/capture/win/video_capture_device_factory_win.cc (right): https://codereview.chromium.org/541603002/diff/20001/media/video/capture/win/video_capture_device_factory_win.cc#newcode131 media/video/capture/win/video_capture_device_factory_win.cc:131: continue; Missing moniker.Release(); https://codereview.chromium.org/541603002/diff/20001/media/video/capture/win/video_capture_device_factory_win.cc#newcode144 media/video/capture/win/video_capture_device_factory_win.cc:144: continue; missing moniker.Release(); https://codereview.chromium.org/541603002/diff/20001/media/video/capture/win/video_capture_device_factory_win.cc#newcode160 ...
6 years, 3 months ago (2014-09-04 16:31:13 UTC) #5
magjed_chromium
Tommi, PTAL, you had some comments on this in another CL. https://codereview.chromium.org/541603002/diff/40001/media/video/capture/win/video_capture_device_factory_win.cc File media/video/capture/win/video_capture_device_factory_win.cc (right): ...
6 years, 3 months ago (2014-09-05 09:15:10 UTC) #7
tommi (sloooow) - chröme
lgtm! https://codereview.chromium.org/541603002/diff/40001/media/video/capture/win/video_capture_device_factory_win.cc File media/video/capture/win/video_capture_device_factory_win.cc (right): https://codereview.chromium.org/541603002/diff/40001/media/video/capture/win/video_capture_device_factory_win.cc#newcode114 media/video/capture/win/video_capture_device_factory_win.cc:114: for (ScopedComPtr<IMoniker> moniker; nice way of scoping the ...
6 years, 3 months ago (2014-09-05 09:26:40 UTC) #8
perkj_chrome
https://codereview.chromium.org/541603002/diff/40001/media/video/capture/win/video_capture_device_factory_win.cc File media/video/capture/win/video_capture_device_factory_win.cc (right): https://codereview.chromium.org/541603002/diff/40001/media/video/capture/win/video_capture_device_factory_win.cc#newcode137 media/video/capture/win/video_capture_device_factory_win.cc:137: static const char kGoogleCameraAdapter[] = "google camera adapter"; On ...
6 years, 3 months ago (2014-09-05 13:52:39 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/magjed@chromium.org/541603002/40001
6 years, 3 months ago (2014-09-05 14:29:27 UTC) #11
commit-bot: I haz the power
Committed patchset #3 (id:40001) as b90b2a16df7e65f1215ba1c9863405dbf6586d01
6 years, 3 months ago (2014-09-05 16:16:29 UTC) #12
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:39:53 UTC) #13
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/dcad7f4830bb4ff1ec0e0f96b9d5fe78ce302cb1
Cr-Commit-Position: refs/heads/master@{#293530}

Powered by Google App Engine
This is Rietveld 408576698