After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 764034 - preset: Use GST_PRESET_PATH as an extension of the system path, not a replacement of the user path
preset: Use GST_PRESET_PATH as an extension of the system path, not a replace...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal blocker
: 1.8.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-03-22 17:10 UTC by Sebastian Dröge (slomo)
Modified: 2016-03-22 17:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
preset: Use GST_PRESET_PATH as an extension of the system path, not a replacement of the user path (5.72 KB, patch)
2016-03-22 17:10 UTC, Sebastian Dröge (slomo)
none Details | Review
preset: Use GST_PRESET_PATH as an extension of the system path, not a replacement of the user path (5.74 KB, patch)
2016-03-22 17:34 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Sebastian Dröge (slomo) 2016-03-22 17:10:24 UTC
See commit message
Comment 1 Sebastian Dröge (slomo) 2016-03-22 17:10:28 UTC
Created attachment 324546 [details] [review]
preset: Use GST_PRESET_PATH as an extension of the system path, not a replacement of the user path

First load all system presets, then all from the environment variable, then
from the app directory, then from the user directory. Any one in the chain
with the highest version completely replaces all previous ones, later ones
with lower versions are merged in without replacing existing presets.

This is basically the same behaviour as before, just that GST_PRESET_PATH is
inserted as another source of directories between the system and app presets.

It was added in ca08af1f17d2ce36b83998a0ba3a7b8bcafd7872, but was
accidentially overriding the user preset path there. Which caused inconsistent
behaviour as new presets were still stored in the system path, just not loaded
from there. Meaning you could store a new preset (in the user path), just for
GstPreset to not find it anymore later (because it only looked in the
GST_PRESET_PATH instead of the user path).
Comment 2 Thibault Saunier 2016-03-22 17:27:40 UTC
Review of attachment 324546 [details] [review]:

Looks good apart from that.

The user dir > app dir > GST_PRESET_PATH > system dir order sounds right to me.

::: gst/gstpreset.c
@@ -412,0 +439,23 @@
+
+    if (have_env) {
+      GList *l;
... 20 more ...

Not freeing the PresetAndVersion structures?
Comment 3 Sebastian Dröge (slomo) 2016-03-22 17:34:59 UTC
Created attachment 324550 [details] [review]
preset: Use GST_PRESET_PATH as an extension of the system path, not a replacement of the user path

First load all system presets, then all from the environment variable, then
from the app directory, then from the user directory. Any one in the chain
with the highest version completely replaces all previous ones, later ones
with lower versions are merged in without replacing existing presets.

This is basically the same behaviour as before, just that GST_PRESET_PATH is
inserted as another source of directories between the system and app presets.

It was added in ca08af1f17d2ce36b83998a0ba3a7b8bcafd7872, but was
accidentially overriding the user preset path there. Which caused inconsistent
behaviour as new presets were still stored in the system path, just not loaded
from there. Meaning you could store a new preset (in the user path), just for
GstPreset to not find it anymore later (because it only looked in the
GST_PRESET_PATH instead of the user path).
Comment 4 Thibault Saunier 2016-03-22 17:37:55 UTC
Review of attachment 324550 [details] [review]:

OK
Comment 5 Sebastian Dröge (slomo) 2016-03-22 17:40:53 UTC
Attachment 324550 [details] pushed as d7c8ce0 - preset: Use GST_PRESET_PATH as an extension of the system path, not a replacement of the user path