Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to 2.9.b forgets mediainfo path value #132

Open
erAck opened this issue Aug 10, 2018 · 6 comments
Open

Upgrade to 2.9.b forgets mediainfo path value #132

erAck opened this issue Aug 10, 2018 · 6 comments

Comments

@erAck
Copy link
Contributor

erAck commented Aug 10, 2018

Upgrading from 2.9.a to 2.9.b forgot the path to mediainfo in the form
LD_LIBRARY_PATH=/somepath/lib /somepath/bin/mediainfo
and kept the entry field filled with just mediainfo instead, consequently the warning icon mark appeared.

Entering the old value again and dry-running Synchronise, mediainfo wasn't warned about anymore and metadata was extracted. Field value was remembered then.

BUT, switching to the Configuration tab and back to the Synchronise tab the field value is lost again, leaving just mediainfo with a warning icon; this also happens after non-dry-run. Effectively one has to enter the correct value before each Synchronise run.

Expected: old value of the path edit field is preserved during upgrade and when edited.

@xbgmsharp
Copy link
Collaborator

The path settings has been remove and need to set separately.

@erAck
Copy link
Contributor Author

erAck commented Sep 3, 2018

A pointer where to set would had been nice.. anyway, found the mentioning of piwigo-videojs/include/admin/admin_photo.php in https://github.com/xbgmsharp/piwigo-videojs/wiki/Using-MediaInfo%2C-ffmpeg-without-installing-it-inside-the-hosting-system but ... seriously? One has to edit the admin_photo.php file now that gets overwritten with each update? Or am I missing something?

@xbgmsharp
Copy link
Collaborator

xbgmsharp commented Sep 4, 2018

You might want to check #130 (comment)

@erAck
Copy link
Contributor Author

erAck commented Sep 4, 2018

If $conf['vjs_mediainfo_dir'] is really meant to contain the directory only then I assume LD_LIBRARY_PATH=/somepath/lib /somepath/bin will not work.

@xbgmsharp
Copy link
Collaborator

I would assume so.

@casatir
Copy link

casatir commented Mar 6, 2019

I am having kind of the same issue after upgrading from 2.8.a to 2.9.b.

Regarding the code, the array_merge function is often used but it seems to me that it is done the wrong way.

For example, in admin/admin_photo.php, after setting the default value of the $sync_options array, at line 84 :
$sync_options = array_merge(unserialize($conf['vjs_sync']), $sync_options,);
should it not be the other way around :
$sync_options = array_merge($sync_options, unserialize($conf['vjs_sync']));
? Because the PHP documentation says :

If two or more array elements have the same key, the last one overrides the others.

So if we want the database options to overwrite the default ones, we should use $conf['vjs_sync'] at last. Am I wrong ?

I switched this at line 84 of admin/admin_photo.php and it just works has previously (at least in the VideoJS tab of the picture/video).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants