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

Relax requirement for livetv series #12125

Conversation

theguymadmax
Copy link
Contributor

Regression from #9297

Changes

  • Set IsSeries back to program.Episode is not null

Issues

Fixes #11976

@crobibero crobibero added the stable backport Backport into the next stable release label Jun 20, 2024
@@ -167,7 +167,7 @@ private static ProgramInfo GetProgramInfo(XmlTvProgram program, ListingsProvider
Overview = program.Description,
ProductionYear = program.CopyrightDate?.Year,
SeasonNumber = program.Episode.Series,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If program.Episode is null this will throw a nullref exception, no? (same for line 155 and 162)
So IsSeries will always be true? (or this function would throw)

Copy link
Contributor Author

@theguymadmax theguymadmax Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @crobibero can speak to this? I see that he had removed some of these null checks from the same commit. I'm not a programmer in any sense of the manner, I just know enough to get myself in trouble.

As long as it's an episode it should be considered a series. This was the behavior before 10.9.Z.

This comment was marked as spam.

@bpauquette
Copy link
Contributor

bpauquette commented Jul 23, 2024

I would like to test this but struggling a bit with process. I am currently getting a build error in jellyfin-packaging when I attempt to build this. How do I compile in the jellyfin repo? Is that documented?

I got instruction from joshua boniface and I simply needed to ./checkout.py master in the jellyfin-packaging. Had to install sudo apt install python3-git. Then it builds on ubuntu noble for me.

@Bond-009
Copy link
Member

I would like to test this but struggling a bit with process. I am currently getting a build error in jellyfin-packaging when I attempt to build this. How do I compile in the jellyfin repo? Is that documented?

https://github.com/jellyfin/jellyfin?tab=readme-ov-file#server-development

@gnattu
Copy link
Member

gnattu commented Jul 23, 2024

Basically you need to do a git rebase release-10.9.z before build.

@numericOverflow
Copy link

FYI - I'm having this same issue this PR is set to fix, so very interested in getting it merged. Right now, no live TV shows get recognized as a series, so can't "record all episodes" for any show. This is a bug and would love to see it fixed as it used to work great.

Copy link
Member

@gnattu gnattu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but I don't believe this is the correct way to to this.

@@ -167,7 +167,7 @@ private static ProgramInfo GetProgramInfo(XmlTvProgram program, ListingsProvider
Overview = program.Description,
ProductionYear = program.CopyrightDate?.Year,
SeasonNumber = program.Episode.Series,
IsSeries = program.Episode.Series is not null,
IsSeries = program.Episode is not null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the correct way to fix this and will not work as what you intend it to. The type definition of program.Episode is not nullable and thus this expression is always true. You are just setting IsSeries == true for everything.

Copy link
Contributor Author

@theguymadmax theguymadmax Jul 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was looking at this PR and thought this would be a suitable workaround. It was my misunderstanding, but it does what I want it to do, just not for the right reason. Thanks.

In Jellyfin XmlTvListingProvider.cs checks Episode for null to determine if the program is part of a series or not:

SeasonNumber = program.Episode?.Series,
IsSeries = program.Episode != null,

The old code initialized XmlTvProgram.Episode to a new XmlTvEpisode in the constructor, so it could never be null. This results in Jellyfin treating all programs (movies and shows) as shows ("Record series" button is active, program is recorded to the shows recording directory if set, etc).

Making Episode nullable and leaving it null until an episode tag is encountered fixes the behavior in Jellyfin, and besides tests in this repo there was nowhere that uses Episode that doesn't already test it for null.

@theguymadmax
Copy link
Contributor Author

I'm sorry but I don't believe this is the correct way to do this.

Thanks for taking a look at this! I suspected that this setup would allow everything to be recorded as a series. I’ve been running the modified code to record a news series without any issues, and another user has also reported successful results.

I reviewed some sample data today with fresh eyes and think I’ve pinpointed the issue, though I’m not entirely sure why it’s happening.

Here’s the data in question:

	<programme start="20240731170000 -0400" stop="20240731180000 -0400" channel="I11.1.21222.zap2it.com">
		<title lang="en">2024 Paris Olympics</title>
		<sub-title lang="en">Women&apos;s Volleyball: U.S. vs. Serbia</sub-title>
		<desc lang="en">The U.S. takes on Serbia in a women&apos;s pool play match.</desc>
		<date>20240731</date>
		<category lang="en">Sports</category>
		<length units="minutes">60</length>
		<icon src="https://zap2it.tmsimg.com/assets/p27804502_b_v13_aa.jpg" />
		<url>https://tvlistings.zap2it.com//overview.html?programSeriesId=SH04875055&amp;tmsId=EP048750550908</url>
		<episode-num system="dd_progid">EP04875055.0908</episode-num>
		<new />
	</programme>
	
	<programme start="20240731070000 -0400" stop="20240731090000 -0400" channel="I15.1.21224.zap2it.com">
		<title lang="en">Good Morning America</title>
		<desc lang="en">Up-to-the-minute news, weather, lifestyle and topical features.</desc>
		<date>20240731</date>
		<category lang="en">News</category>
		<length units="minutes">120</length>
		<icon src="https://zap2it.tmsimg.com/assets/p184220_b_v13_aa.jpg" />
		<url>https://tvlistings.zap2it.com//overview.html?programSeriesId=SH03944032&amp;tmsId=EP039440321175</url>
		<episode-num system="dd_progid">EP03944032.1175</episode-num>
		<new />
	</programme>

The “2024 Paris Olympics” records as a series, but “Good Morning America” does not. The only difference is the presence of the <sub-title> field. When I add a <sub-title> field to the “Good Morning America” entry, Jellyfin correctly identifies it as a series.

I’ll close this PR and post my findings in the issue thread. Hopefully, this information will help someone implement a proper fix.

@bpauquette
Copy link
Contributor

bpauquette commented Aug 1, 2024

Yes, I tested it and it does indeed work fine.
I question the entire reason for not having the record series button.
What is the requirement?
When is a show a series?
When is it not?

Can we agree that if the show has an episode number (<episode-num> in xml data) it is part of a series?

I scanned my current xmltv.xml and all shows have at least one episode-num and some have up to three so this patch is functionally equivalent to always showing the record series button. In other words just set it to true. All shows are parts of series in my current data set. If you want to be weird then just check if there is at least one episode-num.

Here is an example where one show has three episode-nums

<programme start="20240801123000 -0400" stop="20240801133000 -0400" channel="I2.1.30473.zap2it.com"> <title lang="en">The Young and the Restless</title> <desc lang="en">Drama revolves around three families in Genoa City.</desc> <date>20240801</date> <category lang="en">Series</category> <length units="minutes">60</length> <icon src="https://zap2it.tmsimg.com/assets/p183904_b_v13_ai.jpg" /> <url>https://tvlistings.zap2it.com//overview.html?programSeriesId=SH04768071&amp;tmsId=EP047680710288</url> <episode-num system="common">S51E211</episode-num> <episode-num system="dd_progid">EP04768071.0288</episode-num> <episode-num system="xmltv_ns">50.210.</episode-num> <new /> <subtitles type="teletext" /> <rating> <value>TV-14</value> </rating> </programme>

@theguymadmax theguymadmax deleted the livetv-record-series branch August 15, 2024 04:32
@nielsvanvelzen nielsvanvelzen removed the stable backport Backport into the next stable release label Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants