Make omxtypes.h utf-8

Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file contains a character starting at offset 0x70 that is illegal in the current source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@google.com, rtoy@chromium.org
BUG=454858, 637203
Committed: https://chromium.googlesource.com/external/webrtc/deps/third_party/openmax/+/7acede9c039ea5d14cf326f44aad1245b9e674a7

[scottmg, 2016-11-09 00:25:05]

Description was changed from

==========
Make omxtypes.h utf-8

R=rtoy@chromium.org
BUG=454858, 637203
==========

to

==========
Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@chromium.org
BUG=454858, 637203
==========

[scottmg, 2016-11-09 00:25:17]

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

[commit-bot: I haz the power, 2016-11-09 00:25:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-09 00:25:20]

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

[commit-bot: I haz the power, 2016-11-09 00:25:21]

Dry run: CLs for remote refs other than refs/pending/heads/master must contain
NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them

[Raymond Toy (Google), 2016-11-09 00:28:12]

rtoy@google.com changed reviewers:
+ rtoy@google.com

[Raymond Toy (Google), 2016-11-09 00:28:12]

UTF-8 is a good idea, so lgtm.

But why is windows compiling anything in openmax_dl?  This was meant for Android
use, so I'm surprised that you're seeing trouble with this.

[scottmg, 2016-11-09 00:30:33]

On 2016/11/09 00:28:12, Raymond Toy (Google) wrote:
> UTF-8 is a good idea, so lgtm.
> 
> But why is windows compiling anything in openmax_dl?  This was meant for
Android
> use, so I'm surprised that you're seeing trouble with this.

I have no idea! I did notice that in the readme. This is the full output:

[1124->1/1126 ~1] CXX
obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
FAILED:
obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
ninja -t msvc -e environment.x86 --
"c:\src\depot_tools\win_toolchain\vs_files\d5dc33b15d1b2c086f2f6632e2fd15882f80dbd3\VC\bin\amd64_x86/cl.exe"
/nologo /showIncludes /FC
@obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj.rsp
/c ../../third_party/webrtc/common_audio/real_fourier_openmax.cc
/Foobj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
/Fd"obj/third_party/webrtc/common_audio/common_audio_cc.pdb"
c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: error C2220: warning
treated as error - no 'object' file generated
c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).
c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: note: to simplify
migration, consider the temporary use of /Wv:18 flag with the version of the
compiler with which you used to build without warnings


Should that not be getting built?

[scottmg, 2016-11-09 00:33:05]

On 2016/11/09 00:30:33, scottmg wrote:
> On 2016/11/09 00:28:12, Raymond Toy (Google) wrote:
> > UTF-8 is a good idea, so lgtm.
> > 
> > But why is windows compiling anything in openmax_dl?  This was meant for
> Android
> > use, so I'm surprised that you're seeing trouble with this.
> 
> I have no idea! I did notice that in the readme. This is the full output:
> 
> [1124->1/1126 ~1] CXX
> obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> FAILED:
> obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> ninja -t msvc -e environment.x86 --
>
"c:\src\depot_tools\win_toolchain\vs_files\d5dc33b15d1b2c086f2f6632e2fd15882f80dbd3\VC\bin\amd64_x86/cl.exe"
> /nologo /showIncludes /FC
> @obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj.rsp
> /c ../../third_party/webrtc/common_audio/real_fourier_openmax.cc
> /Foobj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> /Fd"obj/third_party/webrtc/common_audio/common_audio_cc.pdb"
> c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: error C2220: warning
> treated as error - no 'object' file generated
> c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The
file
> contains a character starting at offset 0x70 that is illegal in the current
> source character set (codepage 65001).
> c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: note: to simplify
> migration, consider the temporary use of /Wv:18 flag with the version of the
> compiler with which you used to build without warnings
> 
> 
> Should that not be getting built?

https://cs.chromium.org/chromium/src/third_party/webrtc/build/webrtc.gni?rcl=...

It's not ios, it's not arm, and it's not mips...

Should I compile that out on desktop instead then?

[Raymond Toy (Google), 2016-11-09 00:34:05]

On 2016/11/09 00:30:33, scottmg wrote:
> On 2016/11/09 00:28:12, Raymond Toy (Google) wrote:
> > UTF-8 is a good idea, so lgtm.
> > 
> > But why is windows compiling anything in openmax_dl?  This was meant for
> Android
> > use, so I'm surprised that you're seeing trouble with this.
> 
> I have no idea! I did notice that in the readme. This is the full output:
> 
> [1124->1/1126 ~1] CXX
> obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> FAILED:
> obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> ninja -t msvc -e environment.x86 --
>
"c:\src\depot_tools\win_toolchain\vs_files\d5dc33b15d1b2c086f2f6632e2fd15882f80dbd3\VC\bin\amd64_x86/cl.exe"
> /nologo /showIncludes /FC
> @obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj.rsp
> /c ../../third_party/webrtc/common_audio/real_fourier_openmax.cc
> /Foobj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> /Fd"obj/third_party/webrtc/common_audio/common_audio_cc.pdb"
> c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: error C2220: warning
> treated as error - no 'object' file generated
> c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The
file
> contains a character starting at offset 0x70 that is illegal in the current
> source character set (codepage 65001).
> c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: note: to simplify
> migration, consider the temporary use of /Wv:18 flag with the version of the
> compiler with which you used to build without warnings
> 
> 
> Should that not be getting built?

Oh.  Maybe webrtc is using it now. Not sure why though.  The last time I
measured it, the x86 version of the FFT in openmax was slower than the version
in ffmpeg.  And having an fft from open max and one from ffmpeg seems a bit of a
waste.

[Raymond Toy (Google), 2016-11-09 00:35:45]

On 2016/11/09 00:33:05, scottmg wrote:
> On 2016/11/09 00:30:33, scottmg wrote:
> > On 2016/11/09 00:28:12, Raymond Toy (Google) wrote:
> > > UTF-8 is a good idea, so lgtm.
> > > 
> > > But why is windows compiling anything in openmax_dl?  This was meant for
> > Android
> > > use, so I'm surprised that you're seeing trouble with this.
> > 
> > I have no idea! I did notice that in the readme. This is the full output:
> > 
> > [1124->1/1126 ~1] CXX
> > obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> > FAILED:
> > obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> > ninja -t msvc -e environment.x86 --
> >
>
"c:\src\depot_tools\win_toolchain\vs_files\d5dc33b15d1b2c086f2f6632e2fd15882f80dbd3\VC\bin\amd64_x86/cl.exe"
> > /nologo /showIncludes /FC
> >
@obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj.rsp
> > /c ../../third_party/webrtc/common_audio/real_fourier_openmax.cc
> > /Foobj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> > /Fd"obj/third_party/webrtc/common_audio/common_audio_cc.pdb"
> > c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: error C2220: warning
> > treated as error - no 'object' file generated
> > c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The
> file
> > contains a character starting at offset 0x70 that is illegal in the current
> > source character set (codepage 65001).
> > c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: note: to simplify
> > migration, consider the temporary use of /Wv:18 flag with the version of the
> > compiler with which you used to build without warnings
> > 
> > 
> > Should that not be getting built?
> 
>
https://cs.chromium.org/chromium/src/third_party/webrtc/build/webrtc.gni?rcl=...
> 
> It's not ios, it's not arm, and it's not mips...
> 
> Should I compile that out on desktop instead then?

Probably, but I think we should ask the webrtc folks. I don't know what the
intent actually is.

But still lgtm for the utf-8 change, if you want to land this still.

[scottmg, 2016-11-09 00:41:17]

On 2016/11/09 00:35:45, Raymond Toy (Google) wrote:
> On 2016/11/09 00:33:05, scottmg wrote:
> > On 2016/11/09 00:30:33, scottmg wrote:
> > > On 2016/11/09 00:28:12, Raymond Toy (Google) wrote:
> > > > UTF-8 is a good idea, so lgtm.
> > > > 
> > > > But why is windows compiling anything in openmax_dl?  This was meant for
> > > Android
> > > > use, so I'm surprised that you're seeing trouble with this.
> > > 
> > > I have no idea! I did notice that in the readme. This is the full output:
> > > 
> > > [1124->1/1126 ~1] CXX
> > > obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> > > FAILED:
> > > obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> > > ninja -t msvc -e environment.x86 --
> > >
> >
>
"c:\src\depot_tools\win_toolchain\vs_files\d5dc33b15d1b2c086f2f6632e2fd15882f80dbd3\VC\bin\amd64_x86/cl.exe"
> > > /nologo /showIncludes /FC
> > >
> @obj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj.rsp
> > > /c ../../third_party/webrtc/common_audio/real_fourier_openmax.cc
> > >
/Foobj/third_party/webrtc/common_audio/common_audio/real_fourier_openmax.obj
> > > /Fd"obj/third_party/webrtc/common_audio/common_audio_cc.pdb"
> > > c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: error C2220:
warning
> > > treated as error - no 'object' file generated
> > > c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The
> > file
> > > contains a character starting at offset 0x70 that is illegal in the
current
> > > source character set (codepage 65001).
> > > c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: note: to simplify
> > > migration, consider the temporary use of /Wv:18 flag with the version of
the
> > > compiler with which you used to build without warnings
> > > 
> > > 
> > > Should that not be getting built?
> > 
> >
>
https://cs.chromium.org/chromium/src/third_party/webrtc/build/webrtc.gni?rcl=...
> > 
> > It's not ios, it's not arm, and it's not mips...
> > 
> > Should I compile that out on desktop instead then?
> 
> Probably, but I think we should ask the webrtc folks. I don't know what the
> intent actually is.
> 
> But still lgtm for the utf-8 change, if you want to land this still.

OK, I'll land this and then investigate removing from the build separately.
Thanks.

[scottmg, 2016-11-09 00:41:20]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2016-11-09 00:41:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-09 00:41:32]

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

[commit-bot: I haz the power, 2016-11-09 00:41:33]

CLs for remote refs other than refs/pending/heads/master must contain NOTRY=true
and NOPRESUBMIT=true in order for the CQ to process them

[scottmg, 2016-11-09 00:43:45]

xref here: https://bugs.chromium.org/p/chromium/issues/detail?id=663582

[scottmg, 2016-11-09 00:44:28]

On 2016/11/09 00:41:33, commit-bot: I haz the power wrote:
> CLs for remote refs other than refs/pending/heads/master must contain
NOTRY=true
> and NOPRESUBMIT=true in order for the CQ to process them

I'm not sure what that means, but I guess I'll try adding those and see if it
lands that way since it should be very low risk.

[scottmg, 2016-11-09 00:44:38]

Description was changed from

==========
Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@chromium.org
BUG=454858, 637203
==========

to

==========
Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@chromium.org
BUG=454858, 637203
NOTRY=true
NOPRESUBMIT=true
==========

[scottmg, 2016-11-09 00:44:42]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2016-11-09 00:44:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[scottmg, 2016-11-09 00:44:55]

The CQ bit was unchecked by scottmg@chromium.org

[commit-bot: I haz the power, 2016-11-09 00:44:56]

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

[commit-bot: I haz the power, 2016-11-09 00:44:57]

Failed to apply patch for dl/api/omxtypes.h:
While running git apply --index -3 -p1;
  Created missing directory dl/api.
  error: dl/api/omxtypes.h: does not exist in index

Patch:       dl/api/omxtypes.h
Index: dl/api/omxtypes.h
diff --git a/dl/api/omxtypes.h b/dl/api/omxtypes.h
index
b64e779653bb3d5fba8ad29a72d2353798bb955a..b90a17a793ff9a6eb381584a0c39a09bbc0fac2a
100644
--- a/dl/api/omxtypes.h
+++ b/dl/api/omxtypes.h
@@ -2,7 +2,7 @@
  * File: omxtypes.h
  * Brief: Defines basic Data types used in OpenMAX v1.0.2 header files.
  *
- * Copyright © 2005-2008 The Khronos Group Inc. All Rights Reserved. 
+ * Copyright © 2005-2008 The Khronos Group Inc. All Rights Reserved.
  *
  * These materials are protected by copyright laws and contain material 
  * proprietary to the Khronos Group, Inc.  You may use these materials

[scottmg, 2016-11-09 00:45:38]

Or... maybe a `git cl land`?

[scottmg, 2016-11-09 00:45:46]

Description was changed from

==========
Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@chromium.org
BUG=454858, 637203
NOTRY=true
NOPRESUBMIT=true
==========

to

==========
Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@chromium.org
BUG=454858, 637203
==========

[scottmg, 2016-11-09 00:47:44]

On 2016/11/09 00:45:38, scottmg wrote:
> Or... maybe a `git cl land`?

Not that either. Do you know how to land this CL?

----------------------------------
[fix-copyright-sign]c:\src\cr\src\third_party\openmax_dl>git cl land
Using 50% similarity for rename/copy detection. Override with --similarity.
Running presubmit commit checks ...

Presubmit checks passed.
Description:
Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@google.com, rtoy@chromium.org
BUG=454858, 637203

Review URL: https://codereview.webrtc.org/2485303002 .
 dl/api/omxtypes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Counting objects: 5, done.
Delta compression using up to 48 threads.
Compressing objects: 100% (5/5), done.
Writing objects: 100% (5/5), 757 bytes | 0 bytes/s, done.
Total 5 (delta 2), reused 0 (delta 0)
remote: Resolving deltas: 100% (2/2)
remote: Processing changes: refs: 1, done
error: failed to push some refs to
'https://chromium.googlesource.com/external/webrtc/deps/third_party/openmax.git'
Failed to push. If this persists, please file a bug.

[Raymond Toy (Google), 2016-11-09 17:50:17]

Description was changed from

==========
Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@chromium.org
BUG=454858, 637203
==========

to

==========
Make omxtypes.h utf-8

Otherwise, Visual Studio warns:

c:\src\cr\src\third_party\openmax_dl\dl\api\omxtypes.h: warning C4828: The file
contains a character starting at offset 0x70 that is illegal in the current
source character set (codepage 65001).

when building with the /utf8 flag.

R=rtoy@google.com, rtoy@chromium.org
BUG=454858, 637203

Committed:
https://chromium.googlesource.com/external/webrtc/deps/third_party/openmax/+/...
==========

[Raymond Toy (Google), 2016-11-09 17:50:19]

Committed patchset #1 (id:1) manually as
7acede9c039ea5d14cf326f44aad1245b9e674a7 (presubmit successful).

[Raymond Toy (Google), 2016-11-09 18:11:56]

On 2016/11/09 00:45:38, scottmg wrote:
> Or... maybe a `git cl land`?

For the record, 'git cl land' works, but I think chromium committers aren't
webrtc committers, so it won't work in general.

And I don't think there's a CQ, so you can't use that to land changes either.

A bit unfortunate. I don't like seeing these changes showing up as if I did
them; CL authors should get the credit in the commit logs.

[Raymond Toy (Google), 2016-11-11 19:23:51]

On 2016/11/09 18:11:56, Raymond Toy (Google) wrote:
> On 2016/11/09 00:45:38, scottmg wrote:
> > Or... maybe a `git cl land`?
> 
> For the record, 'git cl land' works, but I think chromium committers aren't
> webrtc committers, so it won't work in general.
> 
> And I don't think there's a CQ, so you can't use that to land changes either.
> 
> A bit unfortunate. I don't like seeing these changes showing up as if I did
> them; CL authors should get the credit in the commit logs.

scottmg@ To get this change to chromium, we need a DEPS roll.  Since you're
building this on windows, can you do the DEPS roll?

[scottmg, 2016-11-14 17:10:57]

On 2016/11/11 19:23:51, Raymond Toy (Google) wrote:
> On 2016/11/09 18:11:56, Raymond Toy (Google) wrote:
> > On 2016/11/09 00:45:38, scottmg wrote:
> > > Or... maybe a `git cl land`?
> > 
> > For the record, 'git cl land' works, but I think chromium committers aren't
> > webrtc committers, so it won't work in general.
> > 
> > And I don't think there's a CQ, so you can't use that to land changes
either.
> > 
> > A bit unfortunate. I don't like seeing these changes showing up as if I did
> > them; CL authors should get the credit in the commit logs.
> 
> scottmg@ To get this change to chromium, we need a DEPS roll.  Since you're
> building this on windows, can you do the DEPS roll?

Thanks for landing. Yes, I was planning on doing it at some point, but just
working on the UTF8 thing slowly in the background. Are you in need of a roll
for other reasons?

[Raymond Toy (Google), 2016-11-14 17:46:34]

On 2016/11/14 17:10:57, scottmg (ooo nov11) wrote:
> On 2016/11/11 19:23:51, Raymond Toy (Google) wrote:
> > On 2016/11/09 18:11:56, Raymond Toy (Google) wrote:
> > > On 2016/11/09 00:45:38, scottmg wrote:
> > > > Or... maybe a `git cl land`?
> > > 
> > > For the record, 'git cl land' works, but I think chromium committers
aren't
> > > webrtc committers, so it won't work in general.
> > > 
> > > And I don't think there's a CQ, so you can't use that to land changes
> either.
> > > 
> > > A bit unfortunate. I don't like seeing these changes showing up as if I
did
> > > them; CL authors should get the credit in the commit logs.
> > 
> > scottmg@ To get this change to chromium, we need a DEPS roll.  Since you're
> > building this on windows, can you do the DEPS roll?
> 
> Thanks for landing. Yes, I was planning on doing it at some point, but just
> working on the UTF8 thing slowly in the background. Are you in need of a roll
> for other reasons?

Nope, I don't need an update. At least not for this utf8 issue.