Issue 477653002: Bail out with error instead of dvlog if fail to GetNumberOfCapabilities in Windows video capture st…

Issue 477653002: Bail out with error instead of dvlog if fail to GetNumberOfCapabilities in Windows video capture st… (Closed)

Created:
6 years, 4 months ago by Henrik Grunell

Modified:
6 years, 4 months ago

Reviewers:
DaleCurtis

CC:
chromium-reviews, posciak+watch_chromium.org, feature-media-reviews_chromium.org, wjia+watch_chromium.org, mcasas+watch_chromium.org, mcasas

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Project:
chromium

Visibility:
Public.

More Reviews

Description

Bail out with error instead of dvlog if fail to GetNumberOfCapabilities in Windows video capture start. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289917

Patch Set 1 #

Total comments: 3

Created: 6 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+1 line, -1 line)			Patch
	M	media/video/capture/win/video_capture_device_win.cc	View		1 chunk	+1 line, -1 line	3 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Henrik Grunell

Seems to me and Miguel that we should report an error, which shuts down the ...

6 years, 4 months ago (2014-08-14 15:46:03 UTC) #1

DaleCurtis

This change lgtm, but what about the ones below? https://codereview.chromium.org/477653002/diff/1/media/video/capture/win/video_capture_device_win.cc File media/video/capture/win/video_capture_device_win.cc (right): https://codereview.chromium.org/477653002/diff/1/media/video/capture/win/video_capture_device_win.cc#newcode330 media/video/capture/win/video_capture_device_win.cc:330: ...

6 years, 4 months ago (2014-08-14 18:04:13 UTC) #2

Henrik Grunell

https://codereview.chromium.org/477653002/diff/1/media/video/capture/win/video_capture_device_win.cc File media/video/capture/win/video_capture_device_win.cc (right): https://codereview.chromium.org/477653002/diff/1/media/video/capture/win/video_capture_device_win.cc#newcode330 media/video/capture/win/video_capture_device_win.cc:330: if (FAILED(hr)) On 2014/08/14 18:04:13, DaleCurtis wrote: > Should ...

6 years, 4 months ago (2014-08-15 06:26:37 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/grunell@chromium.org/477653002/1

6 years, 4 months ago (2014-08-15 06:28:30 UTC) #5

Henrik Grunell

6 years, 4 months ago (2014-08-19 06:56:37 UTC) #7

Message was sent while issue was closed.

https://codereview.chromium.org/477653002/diff/1/media/video/capture/win/vide...
File media/video/capture/win/video_capture_device_win.cc (right):

https://codereview.chromium.org/477653002/diff/1/media/video/capture/win/vide...
media/video/capture/win/video_capture_device_win.cc:330: if (FAILED(hr))
On 2014/08/15 06:26:37, Henrik Grunell wrote:
> On 2014/08/14 18:04:13, DaleCurtis wrote:
> > Should this go with the hr = in the if() above? and be replaced with another
> > message as part of an else?
> 
> Yes it seems so. I'll adress this in a follow-up.

FTR, https://codereview.chromium.org/482413002/

Expand Messages | Collapse Messages