RFC: PVR EPG tag "First Aired" value showing up incorrectly in GUI
#1
I wanted to ask here before submitting a PR (I've kept ksooo far too busy the past few days dealing with me) to make sure this is going to be something valid.

In the GUI, when viewing an EPG Tag's "Program Information", the First Aired date is always wrong.  My PVR addon is sending in an accurate UTC value as specified by the PVR API, but my backend always reports this as a date, for example 1578614400 (Friday January 10th, 2020, 12:00:00 AM GMT).  When the tag is processed by CPVRGUIInfo, it's being converted into local time, so being in the Eastern Time zone, all my values are shifted to 7:00PM the previous day, making it incorrect.  Using the 1578614400 example, I will see "First Aired" as 01/09/2020 instead of 01/10/2020.

Here is where the conversion to local time is taking place:

https://github.com/xbmc/xbmc/blob/master...o.cpp#L614

I think this block should be changed as follows, but don't want to submit a PR if this behavior is expected.  I don't want to make any assumptions that would mess up all the official PVR addons.  I can just as easily modify my PVR addon to adjust the value in the other direction before reporting it.

cpp:
      case LISTITEM_PREMIERED:
        if (epgTag->FirstAiredAsUTC().IsValid())
        {
          strValue = epgTag->FirstAiredAsUTC().GetAsLocalizedDate(true);
          return true;
        }
        return false;

Feedback is appreciated, I really don't want to cause ksooo any more grief for a while.
Reply
#2
The screen in your PR look totally different then mine, I see the correct date and time (in long format) with the (New!) marker in blue when appropriate.

I also don't have a year on the title, except with movies where I populate the year field and on those I don't populate original air date.

Martin
Reply
#3
My remaining open PR is for PVR Recordings as opposed to PVR EPG.  If you go into the EPG and right-click/"Programme Information" do you also see the correct "First Aired" date?  I think it would depend on where you are and what GMT adjustment is being made (I'm GMT-05:00, which is why it's showing up as 7:00 PM the prior day), and maybe your backend is giving you a truly accurate date/time stamp as opposed to just the day like mine?  Are you making any adjustments to the time_t value(s) before passing them into Kodi?

This is exactly what I'm worried about with this request, though -- I don't want to mess up anyone else with something I can just as easily adjust on my end.  The PVR API says to use UTC, but when the GUI gets involved, it's being converted to local Sad
Reply
#4
Yes I  see the correct date.   https://imgur.com/peAoB6H

I wish it was localtime but ksooo reiterated it was UTC so until sub fixes the backend I set the time to noon so that it will be correct in Europe and North America.

Martin
Reply
#5
(2020-01-09, 05:55)emveepee Wrote: Yes I  see the correct date.   https://imgur.com/peAoB6H

I wish it was localtime but ksooo reiterated it was UTC so until sub fixes the backend I set the time to noon so that it will be correct in Europe and North America.

Martin
Do you think a PR to change the value to be interpreted as UTC instead of local would negatively impact you?  Sounds like it wouldn't since you're adjusting to noon (believe it or not I hadn't thought of doing that - nice!), but I definitely don't want to put something out there that would screw everyone else up Smile
Reply
#6
I always ignored this field because the source timezone is clearly undefined and the basic c++ timezone handling doesn't give a lot of latitude for adjusting it in the server and having to add CDateTime libraries seemed overkill.  The new isNew flag changed that.  My comments here stand https://github.com/xbmc/xbmc/pull/16645#...-536309917 but I believe defining it localtime is better than hacking the display.

My real issue continues to be the use of firstAired as the New Flag.  You are going to see that Live events from then new TMS data won't have an original air date, and shows can be marked new when not be showing showing on the original air date, because of timezones, alternative countries, movie (ie first network showing). To get the New Flag to show for these show I force firstAired to startTime.  Your proposed conversation could screw that up.   With TMS data all over the world in supported countries an via IPTV EPG lists this becomes a global issue too

The TMS fields to consider for EPG_TAGS tare New, Live, Season Premier/Finale, Series Premier/Finale

Since this is an RFC if you are adjusting it note that linux time is not a good choice for this field. Probably the most interesting use of the field for me would be reruns with TV show predating 1970

Also any change needs to respect a null value of some sort since it is not available for many shows.

Martin
Reply
#7
Agreed.  The implementation seems inconsistent.  Time is always annoying to deal with.  Using time_t as the basis notwithstanding (being able to use dates prior to 1970 would certainly be nice), of course.  I think that's a much bigger proverbial fish to fry.

What if a new boolean field was added to EPG_TAG, perhaps 'isNew', that might be able to make this work out.  The logic in Kodi could check isNew first, and if it's false then fall back to using the firstAired date, if it's valid ... something like (pseudo-code):

cpp:
case LISTITEM_IS_NEW:
      if (item->IsEPG())
      {
        const std:Confusedhared_ptr<CPVREpgInfoTag> epgTag = item->GetEPGInfoTag();

        if (epgTag->IsNew())   // <--- NEW
          return true;

        const CDateTime firstAired = epgTag->FirstAiredAsUTC();
        const CDateTime start = epgTag->StartAsUTC();
        if (firstAired.IsValid() && start.IsValid())
        {
          bValue = firstAired.GetYear() == start.GetYear() &&
                   firstAired.GetMonth() == start.GetMonth() &&
                   firstAired.GetDay() == start.GetDay();
          return true;
        }
      }
      break;

For providers like TMS where the original air date isn't available, but you have the new flag, you could just set that and leave firstAired as zero.  For providers that just supply the original air date, they leave isNew false.  For providers that have both, great!  I think the UI would also have to omit the Original Air Date from things like Program Information if that field isn't valid, which it probably already does.

This kinda reinforces what I was thinking above with a proposed PR to adjust GUIInfo to display the UTC date for first aired in program information, though.  Converting it to local time doesn't make sense when it's being treated as a date, like iYear would be.  It's going to be compared against the UTC startTime, not the TZ adjusted startTime.

Maybe we can propose a new time zone - KST (Kodi Standard Time) Smile
Reply
#8
Also, I don't think the proposed change for the GUI Info to use UTC would have any effect on the trick you're using.  It wouldn't change the value(s) you're providing via EPG_TAG, or any of the logic used to determine "New" in the current code.  It would just change what the user sees in the Program Information dialog.

If I were to modify the firstAired to deal with it being localized, it would screw up the new indicators for me Smile  sigh.  I hate inventing data, it never works out.
Reply
#9
I like being able to use the property to flag new shows and certainly there is is guide date that only provides it.  TMS via Schedules Direct and zap2xml does offer the original air date and the new flag so I can set both. 

The problem scenario is IPTV users in Asia and Oceania, watching North American shows which typically have TMS or TV Passport data in the IPTV EPG guides with North American dates and so if you don't localize them they will be wrong.

Martin
Reply
#10
Do you think we could make the case that firstAired should be specified as a date rather than a time_t (both for EPG and for my Recordings PR).  It think that might give the addon(s) the flexibility they need to deal with their backend data however they need to.

In my case, I'm getting time_t values that are always based at midnight GMT, which is why localizing them messes up the Program Information.  No matter what the time of day is, it will always show the previous day since I'm GMT-5.  If it were a date, though, it would just be whatever I told it to be.  In your case, you could make any conscious adjustments to the date to deal with the North American shows airing overseas.  Not using time_t would also let us send in dates prior to 1/1/1970 and have them show up properly. I'm stuck with time_t from the backend myself for now, but I could ask them to add the date in another format.

If firstAired was a date, I think the logic to determine "isNew" automatically also changes to something more like "compare the firstAired date against the date portion of the localized startTime time_t".  So if the addon said a program first aired today, it would flag as new if the local calendar date is the same.  That's what people want to see, right? They don't care about which side of Greenwich they fall on.

This all falls apart if firstAired is being used somewhere other than for display/informational purposes, though.

If this sounds too dumb or unsellable to ksooo, I can just let him know my Recordings PR needs to change a bit to match EPG before he adds it and figure out a better way to deal with my lack of a granular enough time_t from the backend Smile

I'd still be totally onboard with a separate isNew flag too, I think that would be of benefit.  Give the addon a way to say "hey, this is new even if you don't think it is", but still allows Kodi to do it automatically if it has the information to do so.

BTW - This is fun, thank you for the conversation.  It's very nice to be able to talk something through before making a fool of myself with a PR that would damage things.
Reply
#11
Delete duplicate
Reply
#12
The Original Air Date coming from NextPVR are YYYY-MM-DD direct from the Schedules Direct API so I am flexible, it could even be a text field.   However the API we use is also used with Emby which wants it in UTC, a .NET DateTimeOffset which starts at 1-1-1 but it is also not timezone aware.

The reality is the new flag should have nothing to do with the aired date.

For recordings currently the NextPVR API doesn't even pass the original air date.  The new flag won't be necessary either , it clutters up the screen too since typically users schedule new programs only and they aren't even new by the time I watch them.

I'm glad you opened up a discussion we haven't even tagged this logic for Matrix release yet but I do like see the new indicator.

Martin
Reply
#13
You wanna work on a PR together to see what ksooo thinks? He would probably be more amenable if we can first prove it can be done and works well enough that he wouldn't have to fix a ton of things, if that's how it works out.  I was going to see how hard it would be to change the time_t into a string for my Recordings PR first to get a feel for how well using a string would work with Kodi's CDateTime class.

I think if we succeed, volunteering to help update the other PVR addons would go a long way too.  Changing this would screw up a lot of people's work.
Reply
#14
My track record getting PR's approved and issues addressed is not great, even when I address comments, they just die.   Recently ksooo specifically told me not to work on one I really would like and he would do it when he has time.  Overall Kodi development has not a particularly satisfying experience but I am game if it means getting the isNew flag.  

This is not just a TMS data question there are fields in the XMLTV DTD (previously-shown?, premiere?, last-chance?, new?) that I would like to load with the finale flag even if they are for future use.

Martin
Reply
#15
I think those fields could be added in a non-breaking way with new EPG_TAG_FLAG_IS_XXXX values.  I think "isNew" might be better served there too, actually, since it won't break the API.  I'm going to take a crack at changing firstAired to a string first, since that breaks the API and the database.  I re-submitted my recordings PR with it as a string (W3C; YYYY-MM-DD), seems to work, but hasn't been reviewed yet.

ksooo did mention that I/we/somebody would need to be responsible for fixing all the PVRs if we break something.  Not sure how many support EPG, but it going to be a lot of people to contact.

I'll take my shot at changing to a string, and adding those flags separately since I do think they can be exclusive to one another.  Contact you here with something to review/add-to/etc, or e-mail?

PS - I've certainly had my frustrations with PRs as well, but I try to keep in mind that if somebody PRed code I own I would likely be just, if not moreso, difficult about it.  I always screw up something at least once every single PR and folks like ksooo, FernetMenta, etc have to pull themselves away from what they're working on to review/comment/accept/reject changes that might run counter to another plan, or be so fringe-case that it would feel like a waste of time.  Kodi is HUGE and any change we request might have ramifications we simply didn't see, those guys work pretty hard to prevent that, and I can only imagine how frustrating it can be for them too!
Reply

Logout Mark Read Team Forum Stats Members Help
RFC: PVR EPG tag "First Aired" value showing up incorrectly in GUI0