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

Unified Diff: media/base/media_win.cc

Issue 9450001: Workaround for Windows-only crash inside delay load helper. (Closed) Base URL: http://src.chromium.org/svn/trunk/src/
Patch Set: Created 8 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/base/media_win.cc
===================================================================
--- media/base/media_win.cc (revision 122285)
+++ media/base/media_win.cc (working copy)
@@ -5,6 +5,7 @@
#include "media/base/media.h"
#include <windows.h>
+#include <delayimp.h>
#include "base/file_path.h"
#include "base/logging.h"
@@ -12,6 +13,8 @@
#include "base/native_library.h"
#include "base/path_service.h"
+#pragma comment(lib, "delayimp.lib")
+
namespace media {
enum FFmpegDLLKeys {
@@ -21,18 +24,18 @@
};
// Retrieves the DLLName for the given key.
-static FilePath::CharType* GetDLLName(FFmpegDLLKeys dll_key) {
+static const char* GetDLLName(FFmpegDLLKeys dll_key) {
// TODO(ajwong): Do we want to lock to a specific ffmpeg version?
switch (dll_key) {
case FILE_LIBAVCODEC:
- return FILE_PATH_LITERAL("avcodec-53.dll");
+ return "avcodec-53.dll";
case FILE_LIBAVFORMAT:
- return FILE_PATH_LITERAL("avformat-53.dll");
+ return "avformat-53.dll";
case FILE_LIBAVUTIL:
- return FILE_PATH_LITERAL("avutil-51.dll");
+ return "avutil-51.dll";
default:
LOG(DFATAL) << "Invalid DLL key requested: " << dll_key;
- return FILE_PATH_LITERAL("");
+ return "";
}
}
@@ -44,6 +47,11 @@
if (g_media_library_is_initialized)
return true;
+ // LoadLibraryEx(..., LOAD_WITH_ALTERED_SEARCH_PATH) cannot handle
+ // relative path.
+ if (!base_path.IsAbsolute())
+ return false;
+
FFmpegDLLKeys path_keys[] = {
media::FILE_LIBAVCODEC,
media::FILE_LIBAVFORMAT,
@@ -52,12 +60,12 @@
HMODULE libs[arraysize(path_keys)] = {NULL};
for (size_t i = 0; i < arraysize(path_keys); ++i) {
- FilePath path = base_path.Append(GetDLLName(path_keys[i]));
+ FilePath path = base_path.AppendASCII(GetDLLName(path_keys[i]));
// Use alternate DLL search path so we don't load dependencies from the
// system path. Refer to http://crbug.com/35857
const wchar_t* cpath = path.value().c_str();
- libs[i] = LoadLibraryEx(cpath, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
+ libs[i] = ::LoadLibraryEx(cpath, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
if (!libs[i])
break;
}
@@ -65,17 +73,41 @@
// Check that we loaded all libraries successfully. We only need to check the
// last array element because the loop above will break without initializing
// it on any prior error.
- g_media_library_is_initialized = (libs[arraysize(libs) - 1] != NULL);
+ bool media_library_is_initialized = (libs[arraysize(libs) - 1] != NULL);
- if (!g_media_library_is_initialized) {
+ if (!media_library_is_initialized) {
// Free any loaded libraries if we weren't successful.
for (size_t i = 0; i < arraysize(libs) && libs[i] != NULL; ++i) {
FreeLibrary(libs[i]);
libs[i] = NULL; // Just to be safe.
}
+ return false;
}
- return g_media_library_is_initialized;
+ // Workaround for http://crbug.com/110983
+ // LoadLibrary() sometimes AV's when called by delay load helper when we
+ // call function in ffmpeg for the first time, and we don't know why.
+ // Force delay load helper to fix import table here instead.
+ // Theoretically, there is no need to call LoadLibrary() before
+ // __HrLoadAllImportsForDll(), it will call LoadLibrary() itself, but there
+ // is no way to specify LOAD_WITH_ALTERED_SEARCH_PATH when calling
+ // __HrLoadAllImportsForDll(). So we do everything in 2 steps -- first call
+ // LoadLibraryEx(..., LOAD_WITH_ALTERED_SEARCH_PATH), then call
+ // __HrLoadAllImportsForDll(). Overhead is negligible compared to disk
+ // access time.
+ // Note: in case of error we are not unloading DLL because unload requires
+ // extra resources and should not be necessary; if we ever decide to
+ // unload by calling __FUnloadDelayLoadedDLL() please add /DELAY:UNLOAD
+ // to the linker command line.
+ // TODO(enal): remove that code when we find underlying issue. Delay load
+ // should work if library is alreday in memory, regardless of permissions...
+ for (size_t i = 0; i < arraysize(path_keys); ++i) {
+ if (FAILED(::__HrLoadAllImportsForDll(GetDLLName(path_keys[i]))))
+ media_library_is_initialized = false;
+ }
+
+ g_media_library_is_initialized = media_library_is_initialized;
+ return media_library_is_initialized;
}
void InitializeMediaLibraryForTesting() {
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698