Add ToString function for media::VideoCaptureFormat

Add ToString function for media::VideoCaptureFormat

BUG=409636
Committed: https://crrev.com/e19f20f75e6d0b25a0f9d72a9857475e7d4a6e5c
Cr-Commit-Position: refs/heads/master@{#293332}

[magjed_chromium, 2014-09-01 15:01:37]

magjed@chromium.org changed reviewers:
+ mcasas@chromium.org, perkj@chromium.org

[magjed_chromium, 2014-09-01 15:01:50]

PTAL

[mcasas, 2014-09-01 15:05:56]

We'll get pixel formats as Strings! Yeah!

You should wrap the necessary methods inside a
#if !defined(NDEBUG)
#endif
NDEBUG is false whenever D{,V,P,etc}LOG() are defined.

[magjed_chromium, 2014-09-02 11:06:31]

I tried to wrap the necessary methods inside !defined(NDEBUG), but I got link
errors in release.

[magjed_chromium, 2014-09-03 08:54:35]

magjed@chromium.org changed reviewers:
+ tommi@chromium.org

[magjed_chromium, 2014-09-03 08:56:22]

tommi - Can you please take a look? Especially, what is the proper way to use
#if !defined(NDEBUG) to remove code from release versions?

https://codereview.chromium.org/532533002/diff/80001/media/video/capture/vide...
File media/video/capture/video_capture_types.cc (right):

https://codereview.chromium.org/532533002/diff/80001/media/video/capture/vide...
media/video/capture/video_capture_types.cc:65: #if !defined(NDEBUG)
Do you have some convention on how to do this?

[tommi (sloooow) - chröme, 2014-09-04 08:27:45]

lgtm as discussed

https://codereview.chromium.org/532533002/diff/80001/media/video/capture/vide...
File media/video/capture/video_capture_types.cc (right):

https://codereview.chromium.org/532533002/diff/80001/media/video/capture/vide...
media/video/capture/video_capture_types.cc:65: #if !defined(NDEBUG)
On 2014/09/03 08:56:22, magjed wrote:
> Do you have some convention on how to do this?

assume resolved offline

[magjed_chromium, 2014-09-04 15:37:44]

PTAL

[mcasas, 2014-09-04 16:23:16]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2014-09-04 16:23:17]

lgtm

[commit-bot: I haz the power, 2014-09-04 16:25:03]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/magjed@chromium.org/532533002/140001

[tommi (sloooow) - chröme, 2014-09-04 16:36:19]

took a quick extra look.

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/win...
File media/video/capture/win/video_capture_device_factory_win.cc (right):

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/win...
media/video/capture/win/video_capture_device_factory_win.cc:94: const CLSID
class_id,
const &

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/win...
media/video/capture/win/video_capture_device_factory_win.cc:115: static const
char kGoogleCameraAdapter[] = "google camera adapter";
Can you check with Per if we can remove this?

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/win...
media/video/capture/win/video_capture_device_factory_win.cc:124:
moniker.Release();
no need for this if we're returning... however, is it correct to bail on the
first moniker that fails BindToStorage?  Should we continue?

[mcasas, 2014-09-04 16:38:38]

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/win...
File media/video/capture/win/video_capture_device_factory_win.cc (right):

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/win...
media/video/capture/win/video_capture_device_factory_win.cc:124:
moniker.Release();
On 2014/09/04 16:36:19, tommi wrote:
> no need for this if we're returning... however, is it correct to bail on the
> first moniker that fails BindToStorage?  Should we continue?

We have a separate CL dealing with this very ScopedComPtr
and it also touches the "google camera adapter" char*   :)
http://crrev.com/541603002

[tommi (sloooow) - chröme, 2014-09-04 16:43:35]

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/win...
File media/video/capture/win/video_capture_device_factory_win.cc (right):

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/win...
media/video/capture/win/video_capture_device_factory_win.cc:124:
moniker.Release();
On 2014/09/04 16:38:38, mcasas wrote:
> On 2014/09/04 16:36:19, tommi wrote:
> > no need for this if we're returning... however, is it correct to bail on the
> > first moniker that fails BindToStorage?  Should we continue?
> 
> We have a separate CL dealing with this very ScopedComPtr
> and it also touches the "google camera adapter" char*   :)
> http://crrev.com/541603002

Ah, great.  Saw your comments too in that CL and +1 on those.

[commit-bot: I haz the power, 2014-09-04 18:36:11]

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

[commit-bot: I haz the power, 2014-09-04 18:36:12]

Try jobs failed on following builders:
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)

[mcasas, 2014-09-04 18:38:55]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-09-04 18:40:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/magjed@chromium.org/532533002/140001

[commit-bot: I haz the power, 2014-09-04 20:55:19]

Committed patchset #8 (id:140001) as 6590528521214b6dc489bc75330216a27a9e4fb7

[Sébastien Marchand, 2014-09-05 19:23:09]

sebmarchand@chromium.org changed reviewers:
+ sebmarchand@chromium.org

[Sébastien Marchand, 2014-09-05 19:23:10]

One comment as this is breaking the Win PGO builder :(

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/vid...
File media/video/capture/video_capture_types.cc (right):

https://codereview.chromium.org/532533002/diff/140001/media/video/capture/vid...
media/video/capture/video_capture_types.cc:34: #if !defined(NDEBUG)
I don't think that this prepocessor check is necessary... The unreferenced code
will be automatically removed by the linker so there's no real gain in doing it
manually. 

Also the trick used to ignore the parameters passed to DVLOG & al doesn't seem
to work well for the official builds with some specific optimizations turned
on... Like the PGO, which is currently failing because of this:
http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Win%20PGO%20Builder

We could probably try to fix the EAT_STREAM_PARAMETERS macro:
https://code.google.com/p/chromium/codesearch#chromium/src/base/logging.h&l=426
to support the PGO, but in the meantime I don't see any clear reason to put this
code behind an "#if !defined(NDEBUG)" (but I might be wrong :).

[tommi (sloooow) - chröme, 2014-09-05 19:47:04]

On 2014/09/05 19:23:10, Sébastien Marchand wrote:
> One comment as this is breaking the Win PGO builder :(
> 
>
https://codereview.chromium.org/532533002/diff/140001/media/video/capture/vid...
> File media/video/capture/video_capture_types.cc (right):
> 
>
https://codereview.chromium.org/532533002/diff/140001/media/video/capture/vid...
> media/video/capture/video_capture_types.cc:34: #if !defined(NDEBUG)
> I don't think that this prepocessor check is necessary... The unreferenced
code
> will be automatically removed by the linker so there's no real gain in doing
it
> manually. 
> 
> Also the trick used to ignore the parameters passed to DVLOG & al doesn't seem
> to work well for the official builds with some specific optimizations turned
> on... Like the PGO, which is currently failing because of this:
>
http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Win%20PGO%20Builder
> 
> We could probably try to fix the EAT_STREAM_PARAMETERS macro:
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/logging.h&l=426
> to support the PGO, but in the meantime I don't see any clear reason to put
this
> code behind an "#if !defined(NDEBUG)" (but I might be wrong :).

Hey Sebastien - is there a chance that you (or someone still working) could have
a stab at fixing this?  Magnus is currently traveling and not in a position to
fix this or revert, so if you could help, it would be greatly appreciated.

[Sébastien Marchand, 2014-09-05 19:57:01]

sure, I'll fix this, I just wanted to make sure that there was no objections to
me removing this check. CL's coming with you as a reviewer, or TBR if you
prefer.

[tommi (sloooow) - chröme, 2014-09-05 20:09:58]

On 2014/09/05 19:57:01, Sébastien Marchand wrote:
> sure, I'll fix this, I just wanted to make sure that there was no objections
to
> me removing this check. CL's coming with you as a reviewer, or TBR if you
> prefer.

Feel free to tbr - putting kids to bed etc :)

[commit-bot: I haz the power, 2014-09-10 03:32:32]

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