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

Add support for file based stubs and improve support for disc stubs #7006

Closed
wants to merge 2 commits into from

Conversation

phate89
Copy link
Contributor

@phate89 phate89 commented Apr 22, 2015

This is a different version of #551.

It's the first time I go deep in Kodi code so please be patient with me.
I tried to hide stub files under Kodi vfs in efile:// urls. I used something similar to a mix of Zip and special protocol file for efile stubs.
I also improved disc stubs handling. Now theyworks similarly to iso files. Both they should work better now (thumbs creation, resume support, watched state, right info, episode bookmarks etc etc).

I also removed several CFileItem creations only to get to IsDiscImage with a specific URIUtils method (called also from CFileItem).

I have 3 doubts:

  1. I used FileDirectoryFactory to translate to efile links because is the only way I could find to store an efile:// in the database so I could both get access to the real efile file and the destination one. There are better solutions?
  2. I'm not sure if I have to disable thumbs creation for efile stubs or not. right now is disabled.
  3. (unrelated) In DVDInputStreamBluray before kodi was checking for ".iso|.img" extensions. It's the only place where it doesn't use IsDiscImage that checks also for .nrg files. I replaced it with IsDiscImage. There's a different path for .nrg files I missed?

Needs xcode sync

@phate89
Copy link
Contributor Author

phate89 commented Aug 1, 2015

Is this have a chance to get in kodi core?

return strValue;
}

bool CEFileFile::GetXMLString(const CURL& url, std::string strXMLTag, std::string& strValue)

This comment was marked as spam.

@tamland
Copy link
Member

tamland commented Aug 2, 2015

Is "efile" a commonly used name for this sort of thing or something you invented?:) What does the e stand for?

@phate89 phate89 force-pushed the filestubs branch 3 times, most recently from 711bae1 to 61cf3e7 Compare August 2, 2015 12:53
@phate89
Copy link
Contributor Author

phate89 commented Aug 2, 2015

@tamland thanks for the review.
I agree it's not great to read everytime. The problem is that I can't store it in the database because it needs the xml file there. I can store it in the class but most checks create a new class so it doesn't help. I didn't find a better solution. Is there something in kodi for this kind of caching?
Performance wise it seems it doesn't slow down since with the heaviest task (reading the file) it uses the actual file directly.
About the name I didn't created it. I took #551 pr and tried to fit in kodi vfs. Now they works quite better and support resuming, episode bookmarks, watched state etc etc
I didn't change the names but now this names didn't make sense to me too. Probably something like "media stub" is better..

@tamland
Copy link
Member

tamland commented Aug 2, 2015

Twice then? Once for reading the message when displaying the dialog, once when resolving the path. Remember, it can live on network. I'm not very familiar with these parts, so others will have to comment. Maybe look at how .strm files are handled wrt reading?

CONTROL_ENABLE_ON_CONDITION(ID_BUTTON_PLAY, g_mediaManager.IsDiscInDrive());
if (isDisc)
stubOriginal = URIUtils::GetOpticalMediaPath();
CONTROL_ENABLE_ON_CONDITION(ID_BUTTON_PLAY, CFile::Exists(stubOriginal));

This comment was marked as spam.

This comment was marked as spam.

@phate89
Copy link
Contributor Author

phate89 commented Aug 4, 2015

I checked strm files like @tamland suggested but they are simply playlists. They work similarly to how disc stubs work right now changing the path before to start to play so no resume support, no info, no automatic watched state etc etc.
I did some change and now with my last FileItem change if the file exists it opens the file only twice.. It doesn't seems it's too much of a compromise.. Of course if you have suggestions to avoid that i'll gladly consider them..

@da-anda
Copy link
Member

da-anda commented Jan 22, 2017

@phate89 any plans to resurrect this PR?

@razzeee razzeee added Type: Feature non-breaking change which adds functionality Type: Improvement non-breaking change which improves existing functionality labels Sep 16, 2017
@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Nov 19, 2017
@jenkins4kodi
Copy link
Contributor

@phate89 this needs a rebase

@MartijnKaijser
Copy link
Member

I will close this PR. If there's interest please open a new one (with required changes if needed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rebase needed PR that does not apply/merge cleanly to current base branch Type: Feature non-breaking change which adds functionality Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants