View Issue Details

IDProjectCategoryView StatusLast Update
0030762FPCPackagespublic2017-12-31 19:32
ReporterwpAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platform32 bitOSWin 64bitOS VersionWin 10
Product Version3.0.0Product Build 
Target Version3.2.0Fixed in Version3.1.1 
Summary0030762: Hard-coded DLL path in libvlc.pp
DescriptionThe fpc installation contains in folder packages/libvlc the units required to run the multi-format video player VLC in an fpc or Lazarus program. For this purpose, two DLLs are needed. In Windows, these DLLs are found in the VLC installation folder, "c:\Program files\VideoLAN\VLC". This folder is hard-coded in the unit libvlc.pp as "DefaultLibPath".

This fails, however, if the 32-bit VLC is running on a 64-bit Windows because now the installation folder is "c:\Program files (x86)\VideoLAN\VLC".

The attached patch reads the VLC installation folder from the Windows registry instead of relying on a hard-coded path (the patch is relative to packages\libvlc).
Steps To ReproduceRun the demo program testvlc in packages\libvlc\example. It crashes in case of a 32-bit VLC on 64-bit Windows.
Additional InformationSee also the discussion on http://forum.lazarus.freepascal.org/index.php/topic,34420.0.html
TagsNo tags attached.
Fixed in Revision34866
FPCOldBugId
FPCTarget
Attached Files
  • libvlc.pp.patch (1,527 bytes)
    Index: src/libvlc.pp
    ===================================================================
    --- src/libvlc.pp	(revision 34196)
    +++ src/libvlc.pp	(working copy)
    @@ -32,9 +32,10 @@
       libname = 'libvlc.so.5';
     {$else}
     {$ifdef windows}
    -  DefaultlibPath = 'C:\Program files\Videolan\VLC\';
       corelibname    = 'libvlccore.dll';
       libname        = 'libvlc.dll';
    +var
    +  DefaultlibPath: String;
     {$endif}
     {$endif}
     
    @@ -621,7 +622,11 @@
     implementation
     
     uses
    -  SysUtils, dynlibs;
    +  SysUtils, 
    + {$IFDEF WINDOWS}
    +  windows,
    + {$ENDIF}
    +  dynlibs;
     
     var
       hlib : tlibhandle;
    @@ -1168,5 +1173,36 @@
       pointer(libvlc_playlist_play):=GetProcAddress(hlib,'libvlc_playlist_play');
     end;
     
    +{$IFDEF WINDOWS}
    +function GetVLCLibPath: String;
    +var
    +  Handle: HKEY;
    +  RegType: Integer;
    +  DataSize: Cardinal;
    +  Key: PWideChar;
    +  res: WideString;
    +begin
    +  Result := '';
    +  Key := 'Software\VideoLAN\VLC';
    +  if RegOpenKeyExW(HKEY_LOCAL_MACHINE, Key, 0, KEY_READ, Handle) = ERROR_SUCCESS then
    +  begin
    +    if RegQueryValueExW(Handle, 'InstallDir', nil, @RegType, nil, @DataSize) = ERROR_SUCCESS then
    +    begin
    +      SetLength(res, DataSize div 2);
    +      RegQueryValueExW(Handle, 'InstallDir', nil, @RegType, PByte(@res[1]), @DataSize);
    +      res[DataSize div 2] := '\';
    +    end
    +    else
    +      raise Exception.Create('Error on reading registry');
    +    RegCloseKey(Handle);
    +    Result := UTF8Encode(res);
    +  end;
    +end;
    +{$ENDIF}
     
    +initialization
    +{$IFDEF WINDOWS}
    +  DefaultLibPath := GetVLCLibPath;
    +{$ENDIF}
    +
     end.
    
    libvlc.pp.patch (1,527 bytes)

Relationships

related to 0030838 resolvedMichael Van Canneyt vlc components crashes IDE during design time 

Activities

wp

2016-10-19 17:02

reporter  

libvlc.pp.patch (1,527 bytes)
Index: src/libvlc.pp
===================================================================
--- src/libvlc.pp	(revision 34196)
+++ src/libvlc.pp	(working copy)
@@ -32,9 +32,10 @@
   libname = 'libvlc.so.5';
 {$else}
 {$ifdef windows}
-  DefaultlibPath = 'C:\Program files\Videolan\VLC\';
   corelibname    = 'libvlccore.dll';
   libname        = 'libvlc.dll';
+var
+  DefaultlibPath: String;
 {$endif}
 {$endif}
 
@@ -621,7 +622,11 @@
 implementation
 
 uses
-  SysUtils, dynlibs;
+  SysUtils, 
+ {$IFDEF WINDOWS}
+  windows,
+ {$ENDIF}
+  dynlibs;
 
 var
   hlib : tlibhandle;
@@ -1168,5 +1173,36 @@
   pointer(libvlc_playlist_play):=GetProcAddress(hlib,'libvlc_playlist_play');
 end;
 
+{$IFDEF WINDOWS}
+function GetVLCLibPath: String;
+var
+  Handle: HKEY;
+  RegType: Integer;
+  DataSize: Cardinal;
+  Key: PWideChar;
+  res: WideString;
+begin
+  Result := '';
+  Key := 'Software\VideoLAN\VLC';
+  if RegOpenKeyExW(HKEY_LOCAL_MACHINE, Key, 0, KEY_READ, Handle) = ERROR_SUCCESS then
+  begin
+    if RegQueryValueExW(Handle, 'InstallDir', nil, @RegType, nil, @DataSize) = ERROR_SUCCESS then
+    begin
+      SetLength(res, DataSize div 2);
+      RegQueryValueExW(Handle, 'InstallDir', nil, @RegType, PByte(@res[1]), @DataSize);
+      res[DataSize div 2] := '\';
+    end
+    else
+      raise Exception.Create('Error on reading registry');
+    RegCloseKey(Handle);
+    Result := UTF8Encode(res);
+  end;
+end;
+{$ENDIF}
 
+initialization
+{$IFDEF WINDOWS}
+  DefaultLibPath := GetVLCLibPath;
+{$ENDIF}
+
 end.
libvlc.pp.patch (1,527 bytes)

wp

2016-10-19 17:06

reporter   ~0095221

Aaah, I cannot edit my report here..., but maybe this could cause trouble: the patch was created from fpc trunk, but I had specified version 3.0.0 as Product Version.

Michalis Kamburelis

2016-10-20 21:06

reporter   ~0095239

A small suggestion to the patch: The GetVLCLibPath is run from the unit "initialization" section (so, once it's in your uses clause, you have no way to avoid it or surround it with try..except). I would suggest to make sure it doesn't raise an exception, even in case of weird system configuration.

- If "Software\VideoLAN\VLC" exists in registry, but "Software\VideoLAN\VLC\InstallDir" doesn't (maybe because user messed up his registry by manually editing it by regedit), maybe leave Result as '' instead of raising an exception.

- Ignore the case when DataSize = 0 (right now it may cause problem on "res[DataSize div 2] :=" assignment).

I know, these are both extreme edge cases. But, like I mentioned -- I would be careful to make the "initialization" section of a unit as much robust as possible. This avoids crashing an application that merely uses the LibVLC unit on some weird / half-broken registry state.

Alternatively, it may be simpler to just make the DefaultLibPath a local variable inside Loadlibvlc, and call "DefaultLibPath := GetVLCLibPath;" inside Loadlibvlc. Was it useful to have constant DefaultLibPath public in LibVLC unit?

wp

2016-10-20 21:21

reporter   ~0095240

> Alternatively, it may be simpler to just make the DefaultLibPath a
> local variable inside Loadlibvlc, and call "DefaultLibPath := GetVLCLibPath;"
> inside Loadlibvlc. Was it useful to have constant DefaultLibPath public
> in LibVLC unit?

Yes, a local variable was my first idea, I had dropped it because I thought maybe somebody would need it some time later. But this is not an important reason at all...

jamie philbrook

2016-11-03 02:11

reporter   ~0095507

what's wrong with doing it the way windows does it now?
you set no path to the DLL file, just the "NAME.DLL". The
system first looks in the application folder for it and then
in the system folder for it.

 There is no path conflicts this way.. You simply copy the needed DLL
into your project files and ship it with your project as part of the
install files. You have choice of installing it in the system folder or
leaving it in the app folder..

Thaddy de Koning

2016-11-03 09:44

reporter   ~0095516

You can even use just NAME
FPC resolves if it is .so or .dll as long as NAME is the same for the platforms.
Also note properly installed libraries should NEVER need a full path in production code. Only while debugging.

wp

2016-11-03 10:46

reporter   ~0095517

Last edited: 2016-11-03 11:01

View 3 revisions

> You simply copy the needed DLL into your project files and ship it with your project as part of the install files.

The problem is that the demo testlcl is not running (Message: "Could not create instance of vlc") if the vlc dlls are found inside the exe folder. Only if the installation folder is known then the demo is running fine. No idea what vlc is doing here...


> Also note properly installed libraries should NEVER need a full path in production code. Only while debugging.

Properly installed lib or not - that's the way the vlc windows installer does it. They don't copy anything to system32, syswow64

jamie philbrook

2016-11-06 19:52

reporter   ~0095614

Analyzing your post leads me to believe there are more resources required
for that VLC to work than just a couple of dll's
  
  It is most likely one of the LIBS is looking for more resources using the
path it got loaded with. So the hard coded path points to a presumed installed
VLC player,. etc..
  
  Properly install VLC player should have a register key somewhere pointing
to the host directory and the LIB files should be looking there for the install
folder instead.

wp

2016-11-06 20:06

reporter   ~0095615

> Properly install VLC player should have a register key somewhere pointing
> to the host directory and the LIB files should be looking there for the install
> folder instead.

Exactly. This is what my patch is doing, it is looking in the Windows registry.

Michael Van Canneyt

2016-11-08 13:35

administrator   ~0095661

I did this somewhat differently.
I exposed the function GetVLCLibPath for windows and made sure it raises no exceptions.
I additionally moved the initialization of the directory variable to the loadlibrary call.
The initialization section should remain free of such code.

wp

2016-11-10 15:48

reporter   ~0095738

Still crashing, sorry... Both fpc and lazarus sample projects (testvlc and testlcl, respectively) fail with "Project raised exception class 'External: SIGSEGV'".

Michael Van Canneyt

2016-11-11 10:16

administrator   ~0095750

I tested now on windows.
The testvlc program forgets to load the library, so obviously it will crash.
I adapted it.

tested on windows: before installing vlc, the program gives the expected message, after installing vlc, it works as designed an plays the media file specified on the command-line.

Issue History

Date Modified Username Field Change
2016-10-19 17:02 wp New Issue
2016-10-19 17:02 wp File Added: libvlc.pp.patch
2016-10-19 17:06 wp Note Added: 0095221
2016-10-20 21:06 Michalis Kamburelis Note Added: 0095239
2016-10-20 21:21 wp Note Added: 0095240
2016-11-02 12:27 wp Relationship added related to 0030838
2016-11-03 02:11 jamie philbrook Note Added: 0095507
2016-11-03 09:44 Thaddy de Koning Note Added: 0095516
2016-11-03 10:46 wp Note Added: 0095517
2016-11-03 11:00 wp Note Edited: 0095517 View Revisions
2016-11-03 11:01 wp Note Edited: 0095517 View Revisions
2016-11-04 23:44 Michael Van Canneyt Assigned To => Michael Van Canneyt
2016-11-04 23:44 Michael Van Canneyt Status new => assigned
2016-11-06 19:52 jamie philbrook Note Added: 0095614
2016-11-06 20:06 wp Note Added: 0095615
2016-11-08 13:35 Michael Van Canneyt Fixed in Revision => 34847
2016-11-08 13:35 Michael Van Canneyt Note Added: 0095661
2016-11-08 13:35 Michael Van Canneyt Status assigned => resolved
2016-11-08 13:35 Michael Van Canneyt Fixed in Version => 3.1.1
2016-11-08 13:35 Michael Van Canneyt Resolution open => fixed
2016-11-08 13:35 Michael Van Canneyt Target Version => 3.2.0
2016-11-10 15:48 wp Note Added: 0095738
2016-11-10 15:48 wp Status resolved => feedback
2016-11-10 15:48 wp Resolution fixed => reopened
2016-11-11 10:16 Michael Van Canneyt Fixed in Revision 34847 => 34866
2016-11-11 10:16 Michael Van Canneyt Note Added: 0095750
2016-11-11 10:16 Michael Van Canneyt Status feedback => resolved
2016-11-11 10:16 Michael Van Canneyt Resolution reopened => fixed
2017-12-31 19:32 wp Status resolved => closed