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

Only sort item by width when they have the same path #12626

Merged
merged 2 commits into from
Sep 18, 2024

Conversation

gnattu
Copy link
Member

@gnattu gnattu commented Sep 11, 2024

Since emby era we are re-ordering all media sources by its width when reading from database. However, that would break the sorting made by the media prober when resolving multiple versions from different files. Only do such sorting when media streams share the same path to respect the filename based sorting done by the multi version resolver.

This should have little to no impact for most cases are most of the files will only have strictly one video stream.

Changes

Issues

Fixes #12129

Since emby era we are re-ordering all media sources by its width when reading from database. However, that would break the sorting made by the media prober when resolving multiple versions from different files. Only do such sorting when media streams share the same path to respect the filename based sorting done by the multi version resolver.

This should have little to no impact for most cases are most of the files will only have strictly one video stream.

Signed-off-by: gnattu <gnattuoc@me.com>
Signed-off-by: gnattu <gnattuoc@me.com>
@theguymadmax
Copy link
Contributor

Results:
Screenshot from 2024-09-11 02-58-26

Works as documented based on this test! The results of X-Men Apocalypse (2016).mp4 is probably a not valid test for sorting multiple versions, as files should follow the pattern: "filename - label".

return 1;
}

if (string.Equals(x.Path, y.Path, StringComparison.OrdinalIgnoreCase))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Path is an absolute path, we will never get the same paths for 2 different items. 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except different items will never be sorted like this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can show an example where different items are sorted together by MediaSource?

Copy link
Contributor

@dmitrylyzo dmitrylyzo Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean this block can be safely removed. Currently it doesn't change the order from the prober at all.

"items"="sources"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some containers like Matroska can have multiple video streams in the same file with the same path, and I’m unsure what would happen if we simply remove that part, as it’s been in place since the Emby era. This sorting wasn’t initially added for multi-version handling, but I’m unsure of its original intent. I’m not confident in removing it entirely, as it could potentially break things.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I could imagine that some LiveTV plugins can supply HLS streams with multiple resolution in one MediaSource and having the same URL, which probably relies on this currently.

Copy link
Contributor

@dmitrylyzo dmitrylyzo Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some containers like Matroska can have multiple video streams in the same file with the same path

Oh, those. AFAIK, we don't support multiple video streams. But if it will be implemented in the future, that's fine.

Also I could imagine that some LiveTV plugins can supply HLS streams with multiple resolution in one MediaSource and having the same URL, which probably relies on this currently.

With the current implementation (of the server), they would have to provide them as different items (different Id) with the same path. Here MediaSourceInfo is a "file", not a stream, as I see.

@crobibero crobibero merged commit 97d2f77 into jellyfin:master Sep 18, 2024
18 checks passed
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.

Multiple versions sorts by resolution regardless of naming structure
5 participants