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

Relative paths #659

Merged
merged 75 commits into from
Jan 28, 2022
Merged

Conversation

randomnicode
Copy link
Collaborator

@randomnicode randomnicode commented Nov 4, 2021

This significantly overhauls how paths are stored in the backend in media_files

There is more discussion about it on airsonic/airsonic#165

Previously:

  • media files had unique full paths
  • any cross join across tables was on basis of full paths
  • led to issues when migrating or moving the location of the music folder elsewhere.
    • it would consider the music folder a brand new folder and create new media files with new paths
    • led to loss of statistics per media file as well as accumulated cruft in db.
  • nested music folders aren't a problem
  • podcast folders were not considered part of music folders

Now:

  • media files now have a folder_id and a relative path.
  • the full path is resolved by joining the folder path with the relative path.
  • if the folder path changes, the media file remains unaware and when resolving the full path, utilizes the new folder path, no new db entries are created, and the old data remains
  • there is now a path restriction on music folders so they don't overlap. this is the cost of relative paths. overlapping causes ambiguity in how a particular file may be resolved (using folder 1 or using folder 2)
  • music folders cannot be modified to be in their own hierarchy. if set to an ancestor, it would mean all the relative paths of the children would need to be recalibrated. if set to a descendant, some of the entries (previously children) would be orphaned.
    • instead, a new ancestral or a descendant folder can be created.
  • podcasts are now music folders
  • joins are mostly done on basis of media file ids instead of paths

THIS IS A NON-BACKWARDS COMPATIBLE CHANGE! BACK UP YOUR DATABASE PRIOR TO RUNNING THIS VERSION IN CASE YOU WANT TO REVERT!

@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2021

This pull request introduces 8 alerts when merging 9606ef3 into e70a65a - view on LGTM.com

new alerts:

  • 2 for Uncontrolled data used in path expression
  • 2 for Reference equality test of boxed types
  • 2 for Dereferenced variable may be null
  • 1 for Spurious Javadoc @param tags
  • 1 for Boxed variable is never null

@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2021

This pull request introduces 8 alerts when merging 67119fa into e70a65a - view on LGTM.com

new alerts:

  • 2 for Uncontrolled data used in path expression
  • 2 for Reference equality test of boxed types
  • 2 for Dereferenced variable may be null
  • 1 for Spurious Javadoc @param tags
  • 1 for Boxed variable is never null

@randomnicode
Copy link
Collaborator Author

The effect is the same.

It isn't. As demonstrated by the example above. The former merely stops the indexing of a duplicate path, but it has already been created. The latter, is necessary, but is it sufficient? Are there any other paths that create a media file that might be a duplicate? Seems like createMediaFile is called in a bunch of other places?

Why would anyone want to add a music folder which is already in the hierarchy and as such already part of the collection?

The answer was actually posed in the original manipulation of the question. The issue isn't so much as adding something that's already part of the collection, because you're right, that is indeed redundant. However, a user might want to reduce or narrow the scope of a music folder. He or she may not longer want the entirety of /a/ scanned, instead opting to scan /a/b only.
S/he can do they by (a) editing the existing folder, or (b) by adding a new folder in the hierarchy. In the original formulation, since the hierarchy is limited to one folder per hierarchy, the system needs to adjust in a way such that only one folder remains, whether by absorbing the new created folder or understanding that the user seeks to reduce the scope and modifying the existing folder accordingly.

On the question about adding an ancestor the user can simply edit the path for the music folder instead of adding a new folder

Agreed, adding is a simple enough matter, until you realize that expanding the scope means your existing files are affected because their paths need adjusting also. Imagine a folder /a/b, and a media file in that folder with a relative path c/d.mp3. If you go and edit and expand the scope of the folder to /a now, the relative path of the existing media file needs to change with it and become b/c/d.mp3

Expansion and reduction of scope seem like symmetrical operations.

@Yetangitu
Copy link
Contributor

Yetangitu commented Dec 24, 2021

The effect is the same as in "directories which do not belong to the current folder are excluded". I first put it in getChildrenOf() but later moved it to scanFile() since the former function is called far more often than the latter. Given that the filesystem hierarchy can be traversed without prior scanning if the "fast access" option is not used such a filter does need to live in getChildrenOf().

It should be sufficient from what I see, I've had more than enough acquaintance with createMediaFile() in making the CUE-patch to assume that it is. But... given that the original target was as narrow as allowing users to broaden or narrow the scope of an existing folder I think any solution is overkill - just let them edit the path and do a (full) scan which should mark files outside of scope as unavailable and pick up anything which came into scope after the edit. This is already handled by the code without the need for anything new as far as I know.

@lgtm-com
Copy link

lgtm-com bot commented Jan 4, 2022

This pull request introduces 2 alerts when merging 83de31e into 3d4d0c4 - view on LGTM.com

new alerts:

  • 2 for Dereferenced variable may be null

Yetangitu added a commit to Yetangitu/airsonic-advanced that referenced this pull request Jan 8, 2022
…ded to the database using their relative position in the parent file and are played invidually by means of a "split" transcoding which is inserted into the transcoding chain. Base files - the files from which tracks are indexed - are shown or hidden depending on whether the 'Hide cue-indexed files' option (on the Media Folders settings page) is set or not. Cue-indexed tracks are represented in the database by their path and start position. The start position is stored in a new 'start_position' field in the database with a default value of '-1.0' - negative values indicate non-indexed files.

New settings:

 - Media Folders/Hide cue-indexed files (boolean): show (unset) or hide (set) cue-indexed base files
 - Transcoding/Split command (text): command used to split off section of a file for streaming, the default command `ffmpeg -ss %o -t %d -i %s -vcodec copy -acodec copy -f %f -` achieves this without transcoding (i.e. losless) by splitting on frame boundaries.

This is a continuation of the cue-support patch I submitted to the original airsonic (airsonic/airsonic#856) which never got merged. This version has been refactored for airsonic-advanced, rebased on the to-be-merged relative paths PR (airsonic-advanced#659).

Database changes:

 - add column `start_position` (double) default -1.0 (used to identify and access indexed tracks)
 - add column `index_path` varchar (used to store and identify indexed files)
 - drop unique index on path+folder_id
 - add unique index on path+folder_id+start_position

Rebase on relative-paths

Merge relativepaths
@lgtm-com
Copy link

lgtm-com bot commented Jan 10, 2022

This pull request introduces 2 alerts when merging f1abcc4 into 0a2439e - view on LGTM.com

new alerts:

  • 2 for Dereferenced variable may be null

Yetangitu added a commit to Yetangitu/airsonic-advanced that referenced this pull request Jan 11, 2022
…ded to the database using their relative position in the parent file and are played invidually by means of a "split" transcoding which is inserted into the transcoding chain. Base files - the files from which tracks are indexed - are shown or hidden depending on whether the 'Hide cue-indexed files' option (on the Media Folders settings page) is set or not. Cue-indexed tracks are represented in the database by their path and start position. The start position is stored in a new 'start_position' field in the database with a default value of '-1.0' - negative values indicate non-indexed files.

New settings:

 - Media Folders/Hide cue-indexed files (boolean): show (unset) or hide (set) cue-indexed base files
 - Transcoding/Split command (text): command used to split off section of a file for streaming, the default command `ffmpeg -ss %o -t %d -i %s -vcodec copy -acodec copy -f %f -` achieves this without transcoding (i.e. losless) by splitting on frame boundaries.

This is a continuation of the cue-support patch I submitted to the original airsonic (airsonic/airsonic#856) which never got merged. This version has been refactored for airsonic-advanced, rebased on the to-be-merged relative paths PR (airsonic-advanced#659).

Database changes:

 - add column `start_position` (double) default -1.0 (used to identify and access indexed tracks)
 - add column `index_path` varchar (used to store and identify indexed files)
 - drop unique index on path+folder_id
 - add unique index on path+folder_id+start_position

Rebase on relative-paths

Merge relativepaths
@lgtm-com
Copy link

lgtm-com bot commented Jan 24, 2022

This pull request introduces 2 alerts when merging ba30cf6 into 9c1a7ca - view on LGTM.com

new alerts:

  • 2 for Dereferenced variable may be null

Yetangitu added a commit to Yetangitu/airsonic-advanced that referenced this pull request Jan 24, 2022
…ded to the database using their relative position in the parent file and are played invidually by means of a "split" transcoding which is inserted into the transcoding chain. Base files - the files from which tracks are indexed - are shown or hidden depending on whether the 'Hide cue-indexed files' option (on the Media Folders settings page) is set or not. Cue-indexed tracks are represented in the database by their path and start position. The start position is stored in a new 'start_position' field in the database with a default value of '-1.0' - negative values indicate non-indexed files.

New settings:

 - Media Folders/Hide cue-indexed files (boolean): show (unset) or hide (set) cue-indexed base files
 - Transcoding/Split command (text): command used to split off section of a file for streaming, the default command `ffmpeg -ss %o -t %d -i %s -vcodec copy -acodec copy -f %f -` achieves this without transcoding (i.e. losless) by splitting on frame boundaries.

This is a continuation of the cue-support patch I submitted to the original airsonic (airsonic/airsonic#856) which never got merged. This version has been refactored for airsonic-advanced, rebased on the to-be-merged relative paths PR (airsonic-advanced#659).

Database changes:

 - add column `start_position` (double) default -1.0 (used to identify and access indexed tracks)
 - add column `index_path` varchar (used to store and identify indexed files)
 - drop unique index on path+folder_id
 - add unique index on path+folder_id+start_position

Rebase on relative-paths

Merge relativepaths
@lgtm-com
Copy link

lgtm-com bot commented Jan 27, 2022

This pull request introduces 2 alerts when merging 78dd0bd into 9c1a7ca - view on LGTM.com

new alerts:

  • 2 for Dereferenced variable may be null

@randomnicode randomnicode merged commit f5b0f3c into airsonic-advanced:master Jan 28, 2022
@randomnicode randomnicode deleted the relativepaths branch January 28, 2022 15:25
Yetangitu added a commit to Yetangitu/airsonic-advanced that referenced this pull request Jan 28, 2022
This change adds support for cue-indexed files. Indexed tracks are added to the database using their relative position in the parent file and are played invidually by means of a "split" transcoding which is inserted into the transcoding chain. Base files - the files from which tracks are indexed - are shown or hidden depending on whether the 'Hide cue-indexed files' option (on the Media Folders settings page) is set or not. Cue-indexed tracks are represented in the database by their path and start position. The start position is stored in a new 'start_position' field in the database with a default value of '-1.0' - negative values indicate non-indexed files.

New settings:

 - Media Folders/Hide cue-indexed files (boolean): show (unset) or hide (set) cue-indexed base files
 - Transcoding/Split command (text): command used to split off section of a file for streaming, the default command `ffmpeg -ss %o -t %d -i %s -vcodec copy -acodec copy -f %f -` achieves this without transcoding (i.e. losless) by splitting on frame boundaries.

This is a continuation of the cue-support patch I submitted to the original airsonic (airsonic/airsonic#856) which never got merged. This version has been refactored for airsonic-advanced, rebased on the recently-merged relative paths PR (airsonic-advanced#659), put on display in the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying ‘Beware of the Leopard" to be finally (?) resubmitted as a new PR.

Database changes:

 - add column `start_position` (double) default -1.0 (used to identify and access indexed tracks)
 - add column `index_path` varchar (used to store and identify indexed files)
 - drop unique index on path+folder_id
 - add unique index on path+folder_id+start_position
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

Successfully merging this pull request may close these issues.

None yet

2 participants