changed: Add logic to handle subtitles for stacked files #3819

Merged
merged 1 commit into from Feb 5, 2014

3 participants

@arnova
Team Kodi member

Subtitles for stacked items require additional work besides this but at least one can download subtitles again for each stack item separately. In order to improve this further there are 3 options (I think):

  • Assume subtitle services provide zip's for multipart (aka stacked) movies and act accordingly;
  • Show a dialog for each stack item;
  • Automatically try obtain additional subtitles according to the volume name (eg. for .cd1.srt, also try to download .cd2.srt)

Not sure what the best approach is. @amet / @jmarshallnz: Your thoughts?

@arnova
Team Kodi member
@arnova
Team Kodi member

ping again @jmarshallnz / @amet

@jmarshallnz
Team Kodi member

You don't need the Player.h/cpp changes at all, right?

I dunno what the subs services provide, but I'd tend to just download for all items.

@arnova
Team Kodi member

@jmarshallnz: Those were a remnant of the PR it was based on. Removed it.

@jmarshallnz
Team Kodi member

IMO an attempt to download all subs for the stacked item is the best solution here. If that was done, you wouldn't need the changes to Application.h/cpp as you could do the stack parsing directly I think?

(Perhaps you'd need it to set the current sub once downloaded on the player? Can you get the current file from the player?)

@arnova
Team Kodi member

@jmarshallnz : You can't obtain the current file from the player afaik. That's why I implemented the CApp stuff which basically does the same thing. I'm not sure how the various Python parts exactly work and how easy it would be to modify those to properly handle stacks. Perhaps @amet can have a look at this? Would be nice if we could fix this before Gotham final.

@arnova
Team Kodi member

Since @amet seems to fail to respond after several kind pings. I propose to merge this as is, so at least downloading subs for stacks works again (although not perfect yet).

@arnova
Team Kodi member

ping @jmarshallnz / RM

@jmarshallnz
Team Kodi member

@amet: By the looks the subtitle services grab the first stack item in case they're playing a stack, and it looks like they potentially (well, opensubtitles at least) can return a sub for each item in the stack all at once, or do they tend to only retrieve a single sub, so prefer to be called separately for cd1 and cd2 ?

I think we need a plan here before we add anything. This might be a suitable workaround for Gotham, but we want to make sure we have a plan for how to better handle them in Gotham+1 to avoid changing the API for subtitle services too much (or at least signal in advance what changes are likely).

@arnova
Team Kodi member

@jmarshallnz : Any update on this yet?

@jmarshallnz
Team Kodi member

Apologies for the delay. Agreed it's the only way we'll get something working in Gotham.

What I don't like is the altering of the API which seems a bit messy. I wonder if an alternate might be to add a new function GetCurrentStackItem(). That way the usual GetCurrentFile() etc. is clean, but the subs code can (if a stack) query the appropriate item directly. I think I'd be happy with that.

@arnova
Team Kodi member

@jmarshallnz : Added a commit (will squash later) that implements CApp:CurrentStackItem(). I'm not sure I like it this way. Perhaps make it into a CApp:CurrentUnstackedItem() which returns current item, in case it's not a stack?

@jmarshallnz
Team Kodi member

Yeah, that's better - squash it down and we're good to go.

@arnova
Team Kodi member

@jmarshallnz : Done

@jmarshallnz
Team Kodi member

Thanks! jenkins build this please

@jmarshallnz jmarshallnz merged commit 4e698ae into xbmc:master Feb 5, 2014

1 check passed

Details default Merged build #148 succeeded in 54 min
@arnova arnova deleted the arnova:subtitles_for_stacks branch Nov 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment