Get episode range end from XBMC compatible nfo

[netpok]

Changes
This PR allows parsing the end of multiepisode file from XBMC compatible nfo. This is required because the nfo contains each episode in a root block.

For example:
Stargate Atlantis - 1x01-02:

<episodedetails>
  <title>Rising (1)</title>
  <season>1</season>
  <episode>1</episode>
  ...
</episodedetails>
<episodedetails>
  <title>Rising (2)</title>
  <season>1</season>
  <episode>2</episode>
  ...
</episodedetails>

This resulted the file being parsed as episode 1 instead of episode 1-2.

[daullmer]

What is the result you are expecting?
I've tested this PR with the following file structure:

/Stargate Atlantis
        /Season 01
                 Stargate Atlantis - 1x01-02.mp4
                 Stargate Atlantis - 1x01-02.nfo

and the following as content of the .nfo file:

<episodedetails>
  <title>Rising (1)</title>
  <season>1</season>
  <episode>1</episode>
  <plot>Plot 1</plot>
</episodedetails>
<episodedetails>
  <title>Rising (2)</title>
  <season>1</season>
  <episode>2</episode>
  <plot>Plot 2</plot>
</episodedetails>

This is the result I'm getting with the jf-web:

The episode still shows as episode 1 in the metadata editor

[netpok]

Given the exact same folder structure your sent:

/Stargate Atlantis
        /Season 01
                 Stargate Atlantis - 1x01-02.mkv
                 Stargate Atlantis - 1x01-02.nfo

If the nfo is missing the system pulls in as:

The same with nfo present:

The same with nfo present after this pr:

What I noticed:
Jellyfin handles the episode range with the EpisodeNumberEnd and it actually saves and reads back from the nfo but that is a custom attribute not presented in Kodi (and Sonarr) written nfos.

What else I considered:
Kodi by default handles this problem by adding the same file twice once as 1x01 and once as 1x02, it uses an episode-file mapping so one file can belong to multiple episodes. This would be quite a big change so I opted to just adding the end of range to the current episode as it would happen without the nfo.

What could be added:
Handling episode titles better: currently it takes the title of the first episode just like without nfo.
Alternatively:

• It could take the matching part of both Rising (1) and Rising (2) -> Rising): There could be a case where the two episodes in questions does not match. Also it would need to be trimmed otherwise it could result in something like Rising ().
• It could take both episodes and parse out the numbering (Rising (1) and Rising (2) -> Rising): the problem is that it could contain like Part 1 or similar, it's quite hard to decide what is the title of episode and what is just the part marker. Also there is a possibility for a part 3 and we haven't touched localisations, for example part 1 is 1. rész in my native language.
• We could just concatenate the names like Sonarr does (Rising (1) and Rising (2) -> Rising (1) and Rising (2))

My setup, just for reference:

• Sonarr writes the nfo-s
• Jellyfin set up without metadata providers, just parses the nfo-s
• Kodi clients
• Trakt for keeping tabs what I watched

Reasons of this PR:

• It's nice to have no holes in the collection
• This way Trakt knows that I watched episode 2 too

[daullmer]

Thanks for your detailed explanation!

What would you think about wrapping the original try/catch-block with the while loop (instead of creating a new one) and replacing this line

jellyfin/MediaBrowser.XbmcMetadata/Parsers/EpisodeNfoParser.cs

Line 144 in b646787

item.IndexNumber = num;

with this line

item.IndexNumberEnd = Math.Max(num, item.Item.IndexNumberEnd ?? num); 

?
This would look a bit cleaner IMO but use the plot/artwork information from the last episodedetails tag. I don't use nfo files or multi-episode files personally, so I don't know which episodedetails tag contains the best information.

Nevertheless, I would at least add a comment above the while loop that this is needed because one nfo file can contain multiple episodedetails tags, if it is a multi-episode file.

[crobibero]

Additionally, please add tests for this new nfo parsing case

[netpok]

I think the two xml blocks cannot be easily merged since there could be attributes that shouldn't be overridden (for example displayepisode).

That said I moved the second loop inside the try catch so there is only one block now. Also added some comments and a new test for the feature.

[netpok]

Also I noticed that all the tests have the same typo: succes instead of success, should I make a commit to fix those too?

[crobibero]

Also I noticed that all the tests have the same typo: succes instead of success, should I make a commit to fix those too?

Might as well! Eventually we'll make a pass and fix all of the spelling mistakes that snuck in there

[tamburra]

Would it possible to give the admin the option of what separator to use? I prefer to separate my double episodes with a "space slash space"

"Episode Title 1 / Episode Title 2"

Also is this just for reading preconfigured nfos that are formatted in this way? Is this PR to have Jellyfin write to nfos like this as well when scanning in new media files?

[netpok]

Currently this PR does not merge episode titles since the goal was to match other metadata providers. It certainly can be done but then the other providers should be updated too. I would probably choose | (space - vertical bar - space) as a separator but it could be also added let's say to the advanced settings of the tv show library tag.

Exporting in this format is currently not really feasible because the only thing present for the later episodes are the episode number.

IMHO the only real way to handle this would be to store episodes as a many-to-one relation instead of the current one-to-one or introduce a virtual episode type which points back to the original one. The later option is probably easier to implement with the current database layout. But this should be (and is) a project level decision.

---

I fixed the mentioned typos. If needed I can squash the commits. Also I would love to hear your opinion from the above.