Video thumbs to texture cache #919

Merged
merged 30 commits into from May 8, 2012

Projects

None yet

3 participants

@jmarshallnz
Member

This moves video art to the texture cache.

We store all video art in the video library (for items in the library) in the new art table.

There's backward compat in the thumbloader to find the image. Obviously we may get this wrong in the case where the user has changed the thumb or fanart to something other than the local image or the first scraped image, but that's what this is here to solve :)

It'd be useful to get #918 in first I think.

@Montellese: the last commit uses cached paths for JSON-RPC, as I believe this is required for back-compat. Hopefully we can redo to something like image:// URLs later on so we don't need the texture cache hits (which are on-thread).

@jmarshallnz
Member

Ok, rebased after several changes required.

@Montellese would appreciate you haven't a look over the json-rpc changes. In particular, there's the potential for the videolibrary update routines to update art now as well.

Would really like this to hit May, as quite a few users (particularly those that use mysql) have been waiting on it, so getting it wider tested as I work on the music thumbs would be useful.

@Montellese
Member

I only looked at the last commit and it looks like that should work for now for JSON-RPC. Didn't give it a spin but will be happy to once this has been merged. As there is no pre-frodo addon repo yet there are no webinterfaces and scripts which use the latest changes in JSON-RPC and we can easily fix a bug or two in case it doesn't work as expected.

IMO we should put together a plan on how to proceed on this image-access for JSON-RPC business as it came up in multiple places now and really needs to be addressed.

@jmarshallnz
Member

Thanks @Montellese - I think the plan would be:

  1. Hook up a VFS for image:// URLs.
  2. Replace JSON-RPC urls with image:// URLs. That way the URL itself is controlled by XBMC, so they don't try and grab the uncached images themselves (nothing stopping them decoding it themselves if they really want to though).

With 1 and 2, images may be accessed the exact same way as is done at the moment (backward compat). This would basically run through the texturecache initially at least (ensuring the image is cached before being returned if it hasn't already been done).

  1. Add extras to the image:// to allow resizing/transform.
  2. Create a http api for image access and do whatever is needed to hook that up (internally it would just drop down to use the image:// vfs). This might involve changing what is set in the thumbs/fanart fields - not sure.

Numbers 3 and 4 could be done independently.

@Montellese
Member

Sounds pretty much like what I had in mind. 1 and 2 should not affect JSON-RPC clients as they usually just take what they get from "thumbnail" and/or "fanart" and throw it at http://url:port/vfs/
From what I understand looking at your "hacks" needed for JSON-RPC those two steps are the most important ones and as they won't break backwards compat I say "go for it" :-)

IIRC there was still some discussion on what extras (resizing/transformation) to add to image:// but I'd say the most important one (at least for JSON-RPC clients) is resizing. Once 3 and 4 (github got you there and changed them to 1 and 2) are done we can consider getting rid of the highly insecure VFS access handler in the webserver (which will break backwards compat to Eden).

@jmarshallnz
Member

We have to plan out the API for images quite well - in particular, we need to ensure that whatever we pass in there can't go through the general VFS to end up being a backdoor to access images that the client should not have access to (or worse).

I'm not sure the best way to do that, but I guess one way is to use some sort of ID for the images instead of the image:// URL. This could just be kept in an in-memory map(id,url) easily enough, but would clients expect it to be persistent information (if so, a db map would be needed).

@Montellese
Member

I think we should first implement step 1 and 2 so that the TextureCache is in full control of image access and caching before we start worrying about how we can securely expose that information to the outside world. With the webserver being implemented in handlers it has become a lot easier to write (and replace) specific handlers. Obviously this won't stop us from doing something stupid though ;-)

An image-based ID sounds good to me. The question will be whether JSON-RPC requests will directly return those IDs or if they return a real or an image://-based path which then needs to be passed to Files.PrepareDownload (which already exists for this purpose so looks like we actually did something right in the past) to retrieve the ID which will allow a client to actually download the image.

IMO if a client decides to persistently store media item information it should also store the images (which will also speed up the responsiveness and load time of the client). All other (state-less) clients like the default webinterface will always ask XBMC for the information whenever it is requested by the user and then use whatever is returned right then. So IMO there would be no need to persist those mappings.

@jmarshallnz
Member

Sounds good. JSON-RPC requests would return IDs rather than real paths/images. Otherwise we'd have to add a bunch of checking to the main image API to make sure that the image they're requesting is "allowed". Much easier to give 'em an id as then XBMC is in control of the URLs they have access to.

@jmarshallnz
Member

Ok, rebased the typos etc. that @cptspiff found and rebased on master following @Montellese changes.

Will look into the season thing over the next few days to see whether it's feasible. Otherwise, please continue review as and when you have time :)

@jmarshallnz
Member

Updated with a seasons table so we have a unique id per season, thus m_type is now one of movie,musicvideo,episode,season,tvshow,actor,artist,director,writer,set.

Jonathan Mar... added some commits May 6, 2012
Jonathan Marshall add seasons table to the video database so we have a unique id per se…
…ason
a42a8c1
Jonathan Marshall use the unique id for the season in m_iDbId 95132d6
Jonathan Marshall don't SetCachedVideoThumb after setting the channel icon 58bed30
Jonathan Marshall move embedded video thumbs to the texture cache 8c53e69
Jonathan Marshall remove CThumbLoader::LoadRemoteThumb() now that the texturecache hand…
…les it anyway
3721347
Jonathan Marshall move video (files and embedded) thumbs to the texture cache 6c1daa0
Jonathan Marshall size wasn't stored for picture folder thumbs 93845e2
Jonathan Marshall Add art table to video database 43950fd
Jonathan Marshall CVideoThumbLoader::GetLocalThumb -> CVideoThumbLoader::FillThumb ba2cc0e
Jonathan Marshall CVideoInfoScanner::GetArtwork() needn't download art on thread - cach…
…e them into the texture cache async instead. Further, don't bother setting art if the item already has it.
3f879a7
Jonathan Marshall add m_type attribute to CVideoInfoTag to aid in determining what type…
… of item we have
d414202
Jonathan Marshall move GetArtwork() into the start of AddVideo() c50348c
Jonathan Marshall add routines to get and set art of items 1641d7e
Jonathan Marshall add set/getartforitem methods to the videodb 51e65e1
Jonathan Marshall add art to the video library during scan 34a0715
Jonathan Marshall load video database thumbs in the video thumb loader a9fefed
Jonathan Marshall add backward compatibility to fetch thumbs and fanart for items in th…
…e video library without art assigned
ae9b05a
Jonathan Marshall add ability to send additional elements into CVideoInfoTag::Save a63d6c2
Jonathan Marshall move setting of fanart/thumbs to fetch from and update the db 388cc33
Jonathan Marshall add GetTvShowSeasonArt helper to VideoDatabase 7e42453
Jonathan Marshall when importing and exporting the video library to a single folder str…
…ucture, there's no point in dumping art. Store the URLs in the xml instead
51257ea
Jonathan Marshall fetch season thumbs prior to adding tvshow to the db so that season a…
…rt is available
04af3fd
Jonathan Marshall on scan, add actor thumbs to the db 0dd283a
Jonathan Marshall retreive thumb from db when retrieving cast 5ecba2b
Jonathan Marshall no need to DeleteThumbForItem() in the videodatabase - the texturecac…
…he knows about them
61f1129
Jonathan Marshall get rid of unused video thumb functions 3e7946e
Jonathan Marshall setting of season thumbs can now be handled by the database c05b94f
Jonathan Marshall clear artwork prior to re-fetching info when doing online lookups b01d909
Jonathan Marshall Add backward compatibility for actor/season thumb fetching 870597c
Jonathan Marshall use the cached version of textures for JSON-RPC 2683aef
@jmarshallnz
Member

tested, fixed a few minor issues, rebased again. IMO this is now ready to go.

@jmarshallnz jmarshallnz was assigned May 8, 2012
@ghost
ghost commented May 8, 2012

while granted not a thorough review i have read through it and did not spot anything obvious. show it in there. we will have to deal with it no matter;)

@jmarshallnz
Member

Good enough for me :)

@jmarshallnz jmarshallnz merged commit ed6a28a into xbmc:master May 8, 2012
@Memphiz

on mysql:

ERROR 1170 (42000): BLOB/TEXT column 'media_type' used in key specification without a key length

@tru tru added a commit to plexinc/plex-home-theater-public that referenced this pull request Dec 11, 2013
@tru tru Installer now checks for Vista or later.
Fixes #919
e7e92c5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment