|
|
Chromium Code Reviews|
Created:
8 years, 2 months ago by ddorwin Modified:
8 years, 1 month ago CC:
chromium-reviews Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionFix linking issue with CDM plugin wrapper and pre-built CDM.
BUG=149772
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164868
Patch Set 1 #
Total comments: 4
Patch Set 2 : Addressed feedback. #
Total comments: 2
Patch Set 3 : Removed whitespace. #Messages
Total messages: 13 (0 generated)
I have no idea what's going on here... https://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/wide... File third_party/widevine/cdm/widevine_cdm.gyp (right): https://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/wide... third_party/widevine/cdm/widevine_cdm.gyp:41: #'binaries/win/<(target_arch)/manifest.json', lolwat? You want to check this in this way? You think the CL description adequately describes what you're doing? :) https://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/wide... third_party/widevine/cdm/widevine_cdm.gyp:71: # remove it. Can you point to an upstream compiler (MSVC/gcc/clang) bug?
http://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/widev... File third_party/widevine/cdm/widevine_cdm.gyp (right): http://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/widev... third_party/widevine/cdm/widevine_cdm.gyp:41: #'binaries/win/<(target_arch)/manifest.json', On 2012/10/23 05:52:39, Ami Fischman wrote: > lolwat? You want to check this in this way? You think the CL description > adequately describes what you're doing? :) Done. http://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/widev... third_party/widevine/cdm/widevine_cdm.gyp:71: # remove it. On 2012/10/23 05:52:39, Ami Fischman wrote: > Can you point to an upstream compiler (MSVC/gcc/clang) bug? This was copied from ppapi. It dates back to https://code.google.com/p/ppapi/source/detail?r=2. We'll see if it's necessary for this code. Done.
LGTM
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11227048/4
https://chromiumcodereview.appspot.com/11227048/diff/4/third_party/widevine/c... File third_party/widevine/cdm/widevine_cdm.gyp (right): https://chromiumcodereview.appspot.com/11227048/diff/4/third_party/widevine/c... third_party/widevine/cdm/widevine_cdm.gyp:69: [ 'chromeos == 1 and target_arch == "arm"', { 'git cl patch' shows extra whitespace at the end of this line.
https://chromiumcodereview.appspot.com/11227048/diff/4/third_party/widevine/c... File third_party/widevine/cdm/widevine_cdm.gyp (right): https://chromiumcodereview.appspot.com/11227048/diff/4/third_party/widevine/c... third_party/widevine/cdm/widevine_cdm.gyp:69: [ 'chromeos == 1 and target_arch == "arm"', { On 2012/10/26 21:11:23, Jorge Lucangeli Obes wrote: > 'git cl patch' shows extra whitespace at the end of this line. Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11227048/18001
NOTRY=true'ing b/c this only affects cros/arm, for which there are no default trybots anyway.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11227048/18001
Failed to apply patch for third_party/widevine/cdm/widevine_cdm.gyp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file third_party/widevine/cdm/widevine_cdm.gyp
Hunk #1 FAILED at 14.
Hunk #2 FAILED at 22.
Hunk #3 FAILED at 30.
Hunk #4 FAILED at 38.
Hunk #5 FAILED at 67.
Hunk #6 FAILED at 104.
6 out of 6 hunks FAILED -- saving rejects to file
third_party/widevine/cdm/widevine_cdm.gyp.rej
Patch: third_party/widevine/cdm/widevine_cdm.gyp
Index: third_party/widevine/cdm/widevine_cdm.gyp
diff --git a/third_party/widevine/cdm/widevine_cdm.gyp
b/third_party/widevine/cdm/widevine_cdm.gyp
index
8394954d66624eb3c9d9ba313e33f2ebf5dc630b..57f309f6b6a0232b6596be76dfbaa6b082a60569
100644
--- a/third_party/widevine/cdm/widevine_cdm.gyp
+++ b/third_party/widevine/cdm/widevine_cdm.gyp
@@ -14,7 +14,6 @@
'symbols/chromeos/<(target_arch)/widevine_cdm_version.h',
'widevine_cdm_binary_files%': [
'binaries/chromeos/<(target_arch)/libwidevinecdm.so',
- 'binaries/chromeos/<(target_arch)/manifest.json',
],
}],
[ 'OS == "linux" and chromeos == 0', {
@@ -22,7 +21,6 @@
'symbols/linux/<(target_arch)/widevine_cdm_version.h',
'widevine_cdm_binary_files%': [
'binaries/linux/<(target_arch)/libwidevinecdm.so',
- 'binaries/linux/<(target_arch)/manifest.json',
],
}],
[ 'OS == "mac"', {
@@ -30,7 +28,6 @@
'symbols/mac/<(target_arch)/widevine_cdm_version.h',
'widevine_cdm_binary_files%': [
'binaries/mac/<(target_arch)/libwidevinecdm.dylib',
- 'binaries/mac/<(target_arch)/manifest.json',
],
}],
[ 'OS == "win"', {
@@ -38,7 +35,6 @@
'symbols/win/<(target_arch)/widevine_cdm_version.h',
'widevine_cdm_binary_files%': [
'binaries/win/<(target_arch)/widevinecdm.dll',
- 'binaries/win/<(target_arch)/manifest.json',
],
}],
],
@@ -67,11 +63,14 @@
[ 'os_posix == 1 and OS != "mac"', {
'cflags': ['-fvisibility=hidden'],
'type': 'loadable_module',
- # -gstabs, used in the official builds, causes an ICE. Simply
- # remove it.
- 'cflags!': ['-gstabs'],
# Allow the plugin wrapper to find the CDM in the same directory.
- 'ldflags': ['-Wl,-rpath=\$$ORIGIN']
+ 'ldflags': ['-Wl,-rpath=\$$ORIGIN'],
+ }],
+ [ 'chromeos == 1 and target_arch == "arm"', {
+ 'libraries': [
+ # Copied by widevine_cdm_binaries.
+ '<(PRODUCT_DIR)/libwidevinecdm.so',
+ ],
}],
[ 'OS == "win" and 0', {
'type': 'shared_library',
@@ -104,7 +103,8 @@
'target_name': 'widevine_cdm_binaries',
'type': 'none',
'copies': [{
- # TODO(ddorwin): Do we need a sub-directory?
+ # TODO(ddorwin): Do we need a sub-directory? We either need a
+ # sub-directory or to rename manifest.json before we can copy it.
'destination': '<(PRODUCT_DIR)',
'files': [ '<@(widevine_cdm_binary_files)' ],
}],
|
