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 BDInfo plugin to UHD/Atmos/DTS:X support #1891

Merged
merged 1 commit into from Dec 27, 2019

Conversation

stanionascu
Copy link
Contributor

@stanionascu stanionascu commented Oct 13, 2019

For playback of 1:1 backups via jellyfin, it is required to support not only normal BluRay BDMV folder(s) but also UHD-BD folders. It also keeps the DiscUtils dependency from upstream, so that media info from ISO backups could be extracted.

To make it easier to port later on, the changes from upstream UniqProject/BDInfo are kept to bare minimum. Hence a lot of reverted "fixes".

Changes

Issues
N/A

@JustAMan
Copy link
Contributor

Our current idea is to use as much 3rd party code as possible without including the source.
Can you update the code by removing BDInfo source and instead adding a NuGet dependency on that?

@stanionascu
Copy link
Contributor Author

Makes sense. I'll give it a try.

@KerryRJ
Copy link
Contributor

KerryRJ commented Nov 7, 2019

I would like to know what is meant by "adding a NuGet dependency on that"?
Are you referring to using an upstream UniqProject/BDInfo package, that does not seem to exist?

@dkanada
Copy link
Member

dkanada commented Nov 7, 2019

He probably means use the BDInfo library from NuGet instead of committing it here.

@KerryRJ
Copy link
Contributor

KerryRJ commented Nov 7, 2019

The only BdInfo Nuget package that I can see in Nuget is the Emby package. Is there another one?

@stanionascu
Copy link
Contributor Author

The only BdInfo Nuget package that I can see in Nuget is the Emby package. Is there another one?

I already started looking into creating the Nuget package for the updated BDInfo, but it will still require modifications on top, due to usage of FileSystemMetadata

@stanionascu
Copy link
Contributor Author

I've stripped out BDInfo into a separate library and published it to nuget repo. It worked just fine with the original UI, but I haven't tested it fully with jellyfin yet.

Copy link
Contributor

@JustAMan JustAMan left a comment

Choose a reason for hiding this comment

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

Mostly looks good.

What package targets did you publish on Nuget? Got a link?

Copy link
Contributor

@JustAMan JustAMan left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this!

@anthonylavado
Copy link
Member

Okay, for reference @stanionascu has created this NuGet Package here: https://www.nuget.org/packages/BDInfo/0.7.6.1

@anthonylavado
Copy link
Member

@stanionascu Are you open to transferring the package to us in the Jellyfin organization?

@stanionascu
Copy link
Contributor Author

@stanionascu Are you open to transferring the package to us in the Jellyfin organization?

Sure. I have no problem with that.

@JustAMan
Copy link
Contributor

Do you want to transfer your repo, or want us to fork yours?

@stanionascu
Copy link
Contributor Author

Do you want to transfer your repo, or want us to fork yours?

I'm fine with transferring.

@anthonylavado
Copy link
Member

@stanionascu If you go in to the settings, and transfer it to me, I can then transfer it to the organization.

On NuGet, if you go to "Manage Package", you can add "Jellyfin" as an owner. If that doesn't work, add "anthonylavado" as an owner instead.

@stanionascu
Copy link
Contributor Author

@stanionascu If you go in to the settings, and transfer it to me, I can then transfer it to the organization.

On NuGet, if you go to "Manage Package", you can add "Jellyfin" as an owner. If that doesn't work, add "anthonylavado" as an owner instead.

I've added Jellyfin as nuget package owner, but I can't transfer the ownership due to missing write permissions to the repos. Feel free to fork it instead.

@anthonylavado
Copy link
Member

Repo has been forked and added to the org. Package transfer on NuGet.org complete.

@EraYaN
Copy link
Member

EraYaN commented Dec 17, 2019

We might want to unlink the fork relation if we want search on the repo. (Or ask support to do it)

@anthonylavado
Copy link
Member

We might want to unlink the fork relation if we want search on the repo. (Or ask support to do it)

@EraYaN I can ask support to do it. I don’t think you can do it manually.

@dkanada dkanada merged commit b396305 into jellyfin:master Dec 27, 2019
@stanionascu stanionascu deleted the bdinfo-uhd branch December 28, 2019 19:05
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

7 participants