Skip to content
This repository has been archived by the owner on Apr 15, 2023. It is now read-only.

Add new implementation of the tvheadend addon #370

Closed
wants to merge 208 commits into from

Conversation

Jalle19
Copy link
Collaborator

@Jalle19 Jalle19 commented Oct 20, 2014

I rebased my fork on upstream a few days ago so I decided I may just as well post this as a PR so we can slowly start looking at it. I'm basing this off a separate branch so work can continue in the master branch meanwhile.

There are some few kinks to work out (some Windows-only compiler warnings) and the infamous timeshift bug.

@opdenkamp start reviewing whenever you feel like it. If you want me to squash anything already, please say so.

adamsutton and others added 30 commits February 24, 2014 09:29
This is an attempt to fix several limitations within pvr.hts, and rather than
try and rework broken code I've decided to start from scratch. Some basic
boiler plate has been copied and clearly the code isn't miles different
(since its trying to achieve the same basic goals) but it is a rewrite.
some warnings are still in place since I do intend to use the variables
when I implement the functions. So they will serve as reminders.
…buffer.

I (and others) had completely failed to notice that the default buffer size
for the SyncedBuffer object that pvr.{hts,tvh} was using is ridiculously small
and thus could result in lost data (worse there was no error output - doh).

I have increased to unlimited length, its implicitly bounded by other things
and there is little point bounding it. If it were to fill indefinitely then
something would have to be seriously wrong and the system would probably
eventually crash for other reasons!

This appears to fix timeshift issues, I wonder whether it will fix other things
as well!
opdenkamp and others added 17 commits September 13, 2014 08:18
Fix playback of large recorded mkv files.
…, retention, priority properties of recordings and timers.
…ording-props

[pvr.tvh] Added support for getting and setting startExtra, stopExtra, r...
does) which also makes the "ignore connection lost warnings" actually
work
it's still using the connection (since the connection thread calls into
CTvheadend after it has been destructed)
@opdenkamp
Copy link
Owner

can you squash it and include the removal of pvr.tvh in the same squash. i'm not really interested in the history of what happened after it had been rewritten already. if it were a clean fork that was merged back in, sure, but it's not :)

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Oct 20, 2014

You probably mean the removal of pvr.hts? How do you want to handle that by the way, should we just remove the old addon and keep the name? That way everyone who's using it right now would be able to upgrade to it cleanly.

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Oct 22, 2014

@opdenkamp before I squash, could you take a look at Jalle19@f70feb9 ? This has never really been tested on anything except Linux and Windows so I don't know how the other threading implementations actually handle this.

@opdenkamp
Copy link
Owner

yeah, pvr.hts, not pvr.tvh :)
and seems a lot smarter to just rename the new to pvr.hts indeed instead of using a new name, so users can upgrade without having to change anything. the settings are still the same when i checked. the version number needs to be higher than the current pvr.hts though.

Jalle19@f70feb9 isn't correct, as it doesn't deal with spurious wakes. that's what you need the predicate for. not sure how you're trying to use it now, but PLATFORM::CEvent might be what you need instead.

@opdenkamp
Copy link
Owner

btw, when you said that it was tested on Linux, it wasn't tested very well in that case ;-)

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Oct 24, 2014

We currently use it to wait for a state to change, see https://github.com/Jalle19/xbmc-pvr-addons/blob/342a0334eab21065e970e3129de57777e7619de1/addons/pvr.tvh/src/AsyncState.cpp#L48.

Haven't looked at CEvent, will do.

@opdenkamp
Copy link
Owner

that's double wrong. you must hold the mutex locked before waiting on a PLATFORM:CCondition. the behaviour of this code is undefined.
you should add a boolean 'm_changed' or something like that as predicate and keep the mutex locked when touching it or while waiting for the condition.

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Oct 24, 2014

Okay, I'll look into it.

@opdenkamp
Copy link
Owner

friendly poke @Jalle19 to get the comment addressed and this rebased, so this gets included for I*

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Dec 30, 2014

Thanks, I have one PR pending that I'd like to get merged in one form or another before I rebase and squash this. Hopefully it shouldn't take too long.

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Dec 31, 2014

Replaced by #399

@Jalle19 Jalle19 closed this Dec 31, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
8 participants