Request: Add specific paths to ScanForContent and add a CleanLibrary method for JSON

Also, and this is more of a 'thoughts out loud' type of post, but even if you were to implement a file notification system in XBMC, you would still be required to add path support to the internal functions, yes?

Take inotify for example. Everytime a path changes, it will return the path that changed. You need to do something with that path -- pass it along as a parameter to one of the internal functions of XBMC. Or do you plan to just have XBMC say "oh look, something changed. Scan the whole library!". I guess that is a little better than blindly polling for data (at least it knows when to poll), but it would still be more efficient to pass along the path and only scan that path for new/modified data.

Just a thought -- I haven't really looked into that, so I don't know have valid of an argument it is.

The thing is not about bloat, the thing is that I find it completely unnecessary, you can accomplish all you want with the normal scan. If the only thing that have changed is the path you would have wanted it will pick this up, the only thing that might happen is that you pick up more changed content, which is hardly a bad thing.

So what I'm trying to say is that I don't see any need to expand the method, the scan feature is ofcourse needed and is why its available. so what you want is that when inotify happens, just send scan to jsonrpc and it would work.

What you are proposing would need to be implemented in one of two ways.

1. Any path you give it will scan or clean, despite it being in sources
2. Scan only if given path is in sources
   

Problem with 1 is that every single method has its distinct security, and scanning any path despite it being in sources is a distinctly different security than just scanning what is available, hence it would need to be a different method.

I'm pretty sure you refer to number 2 which would mean jsonrpc need to check if proposed path is in sources, and if the current user has right to alter that source. It will be more code than you think to make it sturdy, and frankly more intelligence than what should be in jsonrpc. (it should ONLY translate from jsonrpc to xbmc core features) so if you implement that security in core, adding the notification stuff for vfs would be a small addition.

What I am trying to say is that all you are trying to do is working around a greater problem which could be fixed much differently. And yes as you say the source is open, feel free to alter your local xbmc and use that I can't and won't stop that but reaching trunk I will not allow, since I see no use for it.

Also I want to point out that the path you provide as a client may not be the same as what xbmc internally uses, another big reason why clients should not and can not provide a proper path.

Say you provide /foo/bar/ we may store this as file:///foo/bar and sure a fuzzy of this would probably pick it up but there are more cases were this applies. And there is already tickets regarding this with httpapi, something I really don't wish to have with jsonrpc. What it provides should be sturdy, really don't care about xbmc internals and hopefully work the same way for a long time.

This really isn't as complicated as you are making it. Let me give you an example. I have network storage where content is stored. I pull content down and immediately store it in the appropriate spot. As soon as it is all in place, I perform a library update and kick off a repack script that queues a lot of tasks (thank you, task spooler for being such a great but simple tool) to transcode the new files. The last task in the queue is another library update. Both library updates use the API to do a general library clean and library update. It is event driven, not polling and can be controlled intelligently.

I fail to see how inotify would add a lot to that particular setup. If I were able to add a path, it might shave a total of 5 seconds of disk activity off of the update. I can live with the 5 seconds of extra activity and I think most other users can, too.

PS Any system that supports curl could do the same. How many systems support XBMC but not curl? None.

First off, I would like to thank you for taking the time to converse with me in the matters of XBMC. Being new to the source and to the language, I am still learning a lot. That said...

topfs2 Wrote:What you are proposing would need to be implemented in one of two ways.

1. Any path you give it will scan or clean, despite it being in sources
2. Scan only if given path is in sources
   

I was actually going to go with 1, but I never thought about the security of it. It wouldn't be too hard for UpdateLibrary() to check if the path is in sources, would it? (I don't know, I haven't looked and don't know where to begin to look) JSON already translates the respective command to the Built-in function, so source-checking isn't something JSON would do. It's something the UpdateLibrary can do if it's given a path parameter.

You say there are so many reasons to NOT include path parameters, and yet when I came along UpdateLibrary(video) has support for a path. Did the XBMC team decide to shift focus on that?

And I wasn't really aiming for this to be included in trunk. I posted it here to get attention of developers and freelancers that would like to help patch in support. If it went into SVN, awesome. If it didn't, then anyone who would want it could apply patches.

I will continue to play around with the source, but the matter is dead in the water as I see it. I'm currently testing out a patch that allows users to clean the music library via the CleanLibrary() function -- currently it only works for video, which makes no sense.

Here's the patch:
http://pastebin.com/UTjuVbmX

Seems to work well, I'll add a Trac Patch ticket later, but while I have your attention, is there any way to clean the music library, but not have that awful dialog box come up? Kinda like clean in the background...?

I'd like to state that I have no problem with the path in core (which as you saw on your ticket I liked), its mainly that I don't see the need for a client to send the path, for many reasons.

And yeah jsonrpc wouldn't technically need to check the path but since its exposing the core method I either need to have jsonrpc do a check, or be garantued that the underlying does it, which I dislike assuming.

So if you want #1 what you really are after is a AddPathToLibrary or AddMediaSource with the possibility to add autoscan. This is a method I think would make sense having as I see more uses in this, for example web gui could allow user to add sources etc. If you still wish to make #2 I would suggest making it to instead of taking a path, take a media source. And by media source I mean an ID which is given in the GetMediaSources method (IIRC the name correct). That way it would perhaps be same security as a full scan, still can't say I perticularily approve of it but that way you have more of a chance for inclusion (perhaps other developers will vote in your favour).

At any rate, when I began this discussion all I wanted was to point out that I think there is a much better way solving the problem, since you run NFS and is tech savvy it may not be perfect but for the vast majority of users a polling solution over NFS is what they would expect, not a script on server telling xbmc what to do.

Thank you for you opinions. It makes a bit more sense to me now and gives me a few more options if I wish to pursue this.

topfs2 Wrote:I'd like to state that I have no problem with the path in core (which as you saw on your ticket I liked)

Yeah, that kinda left me confused. So, you're not against having path parameters in core, but you're against having JSON pass those along. And as I understand it, that's because other core XBMC functions are calling to them properly with proper paths and security built-in, but when you give that privilege to JSON it overrides much of the built-in checks and executes it directly? If that's the case, then I think I can understand the problem now.

I can see what you mean by this is a bigger problem than I initially thought. I'll continue looking at it and hopefully figure out a way to do this efficiently and securely. If anything, this gives me a bit more experience with the language and workings of XBMC, which is only a good thing.

xgamer99 Wrote:Yeah, that kinda left me confused. So, you're not against having path parameters in core, but you're against having JSON pass those along. And as I understand it, that's because other core XBMC functions are calling to them properly with proper paths and security built-in, but when you give that privilege to JSON it overrides much of the built-in checks and executes it directly? If that's the case, then I think I can understand the problem now.

Yeah while the internal method will check if the path is correct and existing I don't like the idea of allowing a client to state which path. While there is a security issue (which in this case is handled already since we assume #2) the biggest problem is that its hard to cover all cases. Like how to format the url properly so its what we store internally.
As an example /media/tvseries/ might have changed but internally we might map this as special://uuid/abs48ds4x57q75x/ when you add it via gui (we don't but there are other feature requests pending that would force us to do). I realise that NFS is not in this perticular case since it will have only one form of URL but the point is that if a method is added it should cover ALL possible cases nicely. Also looking at other feature requests it might be true that NFS would be stored as special://mediasource/X/nfs://192.168.0.2/shows. If sending an ID (which xbmc have given and in NFS case would be X) then it makes more sense, that way you are only scheduleing a pre-mature scan of something we have already planned on scanning. While its probably possible to derive nfs://192.168.0.2/shows from the nfs example, its not magic and code must support this.

So this is why I'd rather see core handle it themself, since A) its more sturdy B) it will benefit more users and would benefit them magically.

If you're just modifying my script xgamer99, to use http calls instead, just know that giving the path wont work as is. Lets say xbmc is already running an update, and you start creating/moving other files around. How would you handle this? If you want to specify the path then you have to make a call for each. In my case (since im scanning the whole library) i only need to do it once.