malformed dvrEntryAdd: 'channel' missing about pvr.hts HOT 21 CLOSED

According to the htsp spec "channel" is required for dvrEntryAdd. So I treat this as a bug in tvheadend.

from pvr.hts.

@adamsutton I have a feeling it was you who added this code, do you know why?

from pvr.hts.

Which code? The one rejecting the malformed dvrEntryAdd? This was me.

from pvr.hts.

BTW: Rejecting should have no side effects, it just does not add/update the "bad" timer/recording to the addons recordings container. => https://github.com/kodi-pvr/pvr.hts/blob/master/src/Tvheadend.cpp#L1754

This should not be the cause for any crash or other bad behavior the user described in the forum. The bad timer/recording will just not be available in Kodi, nothing else.

from pvr.hts.

Yeah the crash the guy was having is unrelated to this, I just wanted to know if this is a potential bug somewhere.

from pvr.hts.

Sweeping this one under the rug, haven't seen it reported anywhere else so I doubt it's actually an issue.

from pvr.hts.

@Jalle19 I must have missed this report, I had the same problem myself and was VERY annoyed by it, I actually fixed it myself. It's caused because of a mistake / change in the HTSP protocol that caused @ksooo to update the addon code to reject messages from older generations of TVH (such as mine!).

However I've now upgraded TVH, because I was interested in seeing what had changed. However I would still fix this if you can. Lots of people will still be using 3.4.

If you re-open I'll take a look.

from pvr.hts.

Well, again a spec vs. reality problem, sigh... So, we need to add another runtime htsp protocol check for Isengard only (as we dropped support for tvh < 4.0 in master).

- if (htsmsg_get_u32(msg, "channel", &channel) && bAdd)
+ if (htsmsg_get_u32(msg, "channel", &channel) && bAdd && (m_conn.GetProtocol() > 10)) /* old tvh's violate spec / spec changed / ... */

from pvr.hts.

@ksooo No, I think I've been through this somewhere else already. I don't believe channel should ever have been listed as "required", it's fundamentally wrong. It's a mistake in the HTSP spec.

Re-open the issue and I'll take a look, be something to do to help get me back into things.

from pvr.hts.

I don't believe channel should ever have been listed as "required", it's fundamentally wrong. It's a mistake in the HTSP spec

Ah okay. Then we would need to fix the code both in master and Isengard, I guess.

Shall I fix the spec? I have write access to tvh wiki.

from pvr.hts.

So the spec is wrong and channel is indeed not required? Sounds like an easy fix.

from pvr.hts.

@adamsutton out of curiosity: why is it fundamentally wrong to have 'channel' required for dvrEntryAdd? What's the use case for a dvrEntry without a channel?

from pvr.hts.

Autorec entries that match title only?

from pvr.hts.

@ksooo because the "channel" field is an identifier for the associated channel object (mapped into Kodi) that a recording was produced on, it's not say a string giving the channel name. This is as it should be, no issue there.

Now imagine I record a whole bunch of shows on a channel I like, let's say "BBC Three", and then the BBC decide to streamline their broadcasting and decide to axe the channel (this is a realistic example), after some time I decide to delete the channel from my TVH instance as it no longer exists. What happens now?

I now have a bunch of perfectly valid recordings, but since the channel they were recorded on no longer exists, dvrEntryAdd no longer set's the channel field (quite right) and kodi then ignores them completely, ouch!

Now I have to remember that this issue exists and that removing a channel would mean I'd have to go "under the hood" and access the PVR files directly, really not nice.

from pvr.hts.

@Jalle19 nope. Autorecs are announced via "autorecEntryAdd", where 'channel" is already optional.

@adamsutton makes sense. Does tvh actually behave like this?

from pvr.hts.

Sounds reasonable, we should drop the requirement then.

from pvr.hts.

@ksooo it certainly used to! And if it doesn't I'll be fixing that as well!

The relevant lines of code should be:

https://github.com/tvheadend/tvheadend/blob/master/src/dvr/dvr_db.c#L485
As long as a channel instance OR name is present, we accept the entry and will store. Even if we can't link the entry to a channel.

https://github.com/tvheadend/tvheadend/blob/master/src/dvr/dvr_db.c#L2306
If a channel is removed, remove all DVR links and set the channel name param in the DVR entry to the name of the channel.

https://github.com/tvheadend/tvheadend/blob/master/src/htsp_server.c#L709
If no channel is associated with the entry, don't add "channel" param, but still output.

I know it worked in the past, as I had recordings associated with deleted channels.

from pvr.hts.

@adamsutton thanks for looking into this. Let's go and fix pvr.hts and the htsp spec, then. :-)

from pvr.hts.

I have a fix ready, just haven't pushed it yet.

from pvr.hts.

@ksooo feel free to update the wiki

from pvr.hts.

feel free to update the wiki

done. https://tvheadend.org/projects/tvheadend/wiki/Htsp/155

from pvr.hts.