Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(594)

Issue 11227048: Fix linking issue with CDM plugin wrapper and pre-built CDM. (Closed)

Created:
8 years, 2 months ago by ddorwin
Modified:
8 years, 1 month ago
CC:
chromium-reviews
Visibility:
Public.

Description

Fix 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. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -9 lines) Patch
M third_party/widevine/cdm/widevine_cdm.gyp View 1 2 6 chunks +9 lines, -9 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
ddorwin
8 years, 2 months ago (2012-10-23 04:04:04 UTC) #1
ddorwin
8 years, 2 months ago (2012-10-23 04:04:34 UTC) #2
Ami GONE FROM CHROMIUM
I have no idea what's going on here... https://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/widevine_cdm.gyp File third_party/widevine/cdm/widevine_cdm.gyp (right): https://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/widevine_cdm.gyp#newcode41 third_party/widevine/cdm/widevine_cdm.gyp:41: #'binaries/win/<(target_arch)/manifest.json', ...
8 years, 2 months ago (2012-10-23 05:52:39 UTC) #3
ddorwin
http://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/widevine_cdm.gyp File third_party/widevine/cdm/widevine_cdm.gyp (right): http://codereview.chromium.org/11227048/diff/1/third_party/widevine/cdm/widevine_cdm.gyp#newcode41 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? ...
8 years, 2 months ago (2012-10-23 15:48:33 UTC) #4
ddorwin
8 years, 2 months ago (2012-10-23 15:48:35 UTC) #5
Ami GONE FROM CHROMIUM
LGTM
8 years, 2 months ago (2012-10-23 15:55:22 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11227048/4
8 years, 2 months ago (2012-10-23 22:22:20 UTC) #7
Jorge Lucangeli Obes
https://chromiumcodereview.appspot.com/11227048/diff/4/third_party/widevine/cdm/widevine_cdm.gyp File third_party/widevine/cdm/widevine_cdm.gyp (right): https://chromiumcodereview.appspot.com/11227048/diff/4/third_party/widevine/cdm/widevine_cdm.gyp#newcode69 third_party/widevine/cdm/widevine_cdm.gyp:69: [ 'chromeos == 1 and target_arch == "arm"', { ...
8 years, 1 month ago (2012-10-26 21:11:23 UTC) #8
ddorwin
https://chromiumcodereview.appspot.com/11227048/diff/4/third_party/widevine/cdm/widevine_cdm.gyp File third_party/widevine/cdm/widevine_cdm.gyp (right): https://chromiumcodereview.appspot.com/11227048/diff/4/third_party/widevine/cdm/widevine_cdm.gyp#newcode69 third_party/widevine/cdm/widevine_cdm.gyp:69: [ 'chromeos == 1 and target_arch == "arm"', { ...
8 years, 1 month ago (2012-10-26 21:31:29 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11227048/18001
8 years, 1 month ago (2012-10-30 06:33:46 UTC) #10
Ami GONE FROM CHROMIUM
NOTRY=true'ing b/c this only affects cros/arm, for which there are no default trybots anyway.
8 years, 1 month ago (2012-10-30 06:39:23 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11227048/18001
8 years, 1 month ago (2012-10-30 06:43:28 UTC) #12
commit-bot: I haz the power
8 years, 1 month ago (2012-10-30 06:43:29 UTC) #13
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)' ],
       }],

Powered by Google App Engine
This is Rietveld 408576698