Change the functions in video_capture_device to pass around PowerLineFrequency enums instead of int…

Change the functions in video_capture_device to pass around PowerLineFrequency enums instead of integers to reduce the amount of static casting required

BUG=525167
Committed: https://crrev.com/8c98c566c838ec5f22964ff066a3e16262023f64
Cr-Commit-Position: refs/heads/master@{#352234}

[suvanjanmukherjee, 2015-09-16 01:03:37]

suvanjanmukherjee@gmail.com changed reviewers:
+ ajose@chromium.org, dcheng@chromium.org, wolenetz@chromium.org

[suvanjanmukherjee, 2015-09-16 01:05:07]

Started on a new branch hence no longer using
https://codereview.chromium.org/1323093006/

Kept the PowerLineFrequency as an enum class as suggested by dcheng. Changed the
functions in video_capture_device to pass around the enum class to reduce the
amount of static casting required.

Ran base and media unittests which pass.

[dcheng, 2015-09-17 00:19:52]

lgtm

[ajose, 2015-09-17 19:33:31]

ajose@chromium.org changed reviewers:
+ mcasas@chromium.org

[ajose, 2015-09-17 19:33:32]

Looking good!

+mcasas@ want to glance at this?

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/video_capture_device_linux.cc (right):

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/video_capture_device_linux.cc:136: PowerLineFrequency
frequency) {
const

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/mac/vid...
File media/capture/video/mac/video_capture_device_mac.mm (right):

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/mac/vid...
media/capture/video/mac/video_capture_device_mac.mm:276: if (frequency
Can this be one line?

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/mac/vid...
media/capture/video/mac/video_capture_device_mac.mm:280: DVLOG(1) << "Setting
Power Line Frequency to " << 0 << " Hz, device "
Update this to log the frequency

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

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/win/vid...
media/capture/video/win/video_capture_device_win.cc:544:
media::PowerLineFrequency::FREQUENCY_50HZ &&
Can these be one line? Do you have access to "git cl format"?

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/win/vid...
media/capture/video/win/video_capture_device_win.cc:565: : 2;
These two can fit on line above?

[wolenetz, 2015-09-17 20:47:49]

media/base lgtm
media/capture/video: I'm leaving full review to OWNERs of that code

[mcasas, 2015-09-21 15:29:46]

The CQ bit was checked by mcasas@chromium.org to run a CQ dry run

[mcasas, 2015-09-21 15:30:07]

A few comments. Ran the trybots for you.

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/video_capture_device_linux.cc (right):

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/video_capture_device_linux.cc:136: PowerLineFrequency
frequency) {
l.136 should be justified two further spaces right.

(Alternatively just run git cl format before uploading).

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/video_c...
File media/capture/video/video_capture_device.cc (right):

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/video_c...
media/capture/video/video_capture_device.cc:143: std::string current_country =
base::CountryCodeForCurrentTimezone();
Can be const, right?

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/video_c...
media/capture/video/video_capture_device.cc:145: return
media::PowerLineFrequency::FREQUENCY_DEFAULT;
No need for media::

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

https://codereview.chromium.org/1345993002/diff/1/media/capture/video/win/vid...
media/capture/video/win/video_capture_device_win.cc:544:
media::PowerLineFrequency::FREQUENCY_50HZ &&
On 2015/09/17 19:33:32, ajose wrote:
> Can these be one line? Do you have access to "git cl format"?

Yeah, git cl format can be run by anyone all the time, and
it only applies to your diff lines (perhaps the immediate 
vicinity as well).

[commit-bot: I haz the power, 2015-09-21 15:30:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1345993002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1345993002/1

[commit-bot: I haz the power, 2015-09-21 16:56:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-21 16:56:04]

Dry run: This issue passed the CQ dry run.

[suvanjanmukherjee, 2015-09-27 13:22:58]

Ran git cl format. Also incorporated suggestions from ajose and mcasas to add
const and fix the debug message.

[ajose, 2015-09-29 22:10:14]

lgtm % nit

https://codereview.chromium.org/1345993002/diff/20001/media/capture/video/mac...
File media/capture/video/mac/video_capture_device_mac.mm (right):

https://codereview.chromium.org/1345993002/diff/20001/media/capture/video/mac...
media/capture/video/mac/video_capture_device_mac.mm:277: ==
PowerLineFrequency::FREQUENCY_DEFAULT)
1 line?

[suvanjanmukherjee, 2015-10-01 23:39:32]

Fixed formatting issue pointed out by ajose. Waiting for approval from mcasas
before I can submit

[mcasas, 2015-10-02 19:02:41]

LGTM, thanks for tackling this!

https://chromiumcodereview.appspot.com/1345993002/diff/40001/media/capture/vi...
File media/capture/video/video_capture_device.cc (right):

https://chromiumcodereview.appspot.com/1345993002/diff/40001/media/capture/vi...
media/capture/video/video_capture_device.cc:143: const std::string
current_country = base::CountryCodeForCurrentTimezone();
Nice!

[suvanjanmukherjee, 2015-10-03 02:13:40]

The CQ bit was checked by suvanjanmukherjee@gmail.com

[suvanjanmukherjee, 2015-10-03 02:13:40]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org,
wolenetz@chromium.org, ajose@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1345993002/#ps40001
(title: "Fixed formatting issue pointed out of ajose")

[commit-bot: I haz the power, 2015-10-03 02:14:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1345993002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1345993002/40001

[commit-bot: I haz the power, 2015-10-03 03:44:00]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-03 03:45:03]

Patchset 3 (id:??) landed as
https://crrev.com/8c98c566c838ec5f22964ff066a3e16262023f64
Cr-Commit-Position: refs/heads/master@{#352234}