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

Timeshift: Seeking fixes #155

Merged
merged 6 commits into from Jan 28, 2016
Merged

Timeshift: Seeking fixes #155

merged 6 commits into from Jan 28, 2016

Conversation

perexg
Copy link
Contributor

@perexg perexg commented Dec 30, 2015

Cherry picked changes from PR #146 . I've not tested compilation on my dev platform (using 15.2), so little tweaking may be required.

Note: Test with TVH v4.1-1264-g0f069c7 and later.
Note2: Another bunch of timeshift fixes TVH v4.1-1276-ga27ed2e (mainly wrong refcounting and reworked on-demand mode).
Note3: TVH v4.1-1299-g80a50ce seems to catch all major timeshift bugs.

@perexg perexg changed the title Seeking fixes Timeshift: Seeking fixes Dec 30, 2015
@Glenn-1990
Copy link
Contributor

Did some tests and it seems working better than before ;-)

@ksooo, shouldn't there be time-shift indicators present in the GUI? I thought that this was implemented in Jarvis but I can't see them.
Also It's not possible to forward/rewind time-shift when there is no epg info, a critical bug in kodi? :s

@ksooo
Copy link
Member

ksooo commented Jan 2, 2016

Did some tests and it seems working better than before ;-)

Cool. Could you please elaborate what "better" means. Especially, what does not yet work?

shouldn't there be time-shift indicators present in the GUI? I thought that this was implemented in Jarvis but I can't see them.

Core (incl. pvr.hts) support was implemented for Jarvis, but no Confluence UI. There was a lengthy discussion in the forum about that and people did not like what @FernetMenta proposed and "decided" to live with nothing instead of something semi-optimal (from their POW). I like what Fernet proposed and use it with my private Jarvis build for months, btw. => You may want to give https://github.com/ksooo/xbmc/commits/timeshift-confluence a try.

Also It's not possible to forward/rewind time-shift when there is no epg info, a critical bug in kodi?

Sounds strange. No idea. Not my area of expertise, sorry.

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 3, 2016

Sounds strange. No idea. Not my area of expertise, sorry.

It's true, it's been like that since forever.

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 3, 2016

Tested with tvheadend 4.0.8, skipping now works reliably. There's still some issues with fast-forwarding/rewinding but I haven't tried tvheadend 4.1 yet so that could be a non-issue too. I suggest we merge this.

@Jalle19 Jalle19 added the Fix label Jan 3, 2016
@ksooo
Copy link
Member

ksooo commented Jan 3, 2016

Tested with tvheadend 4.0.8, skipping now works reliably.

Also tested with 4.0.8. What shall I say? I was timeshifting and the first skip forward I did (no stop before), led to a hang of kodi. :-/

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 3, 2016

That's weird. On-demand disabled?

@ksooo
Copy link
Member

ksooo commented Jan 3, 2016

Yep. Also RAM for buffer is disabled (if this makes a difference).

But looking at the tons of changes and fixes @perexg currently makes on tvh master for the timeshift code and as he wrote that one should test against very latest tvh 4.1 I do not wonder that 4.0.8 does not really work.

@Glenn-1990
Copy link
Contributor

@perexg tvh master seems completely broken now, tvh will hang when using timeshift?

@perexg
Copy link
Contributor Author

perexg commented Jan 4, 2016

Many timeshift related changes are in tvh master - things are stable in my testing environment. I might probably backport the timeshift changes to 4.0.x tree, but I'd like to test it more intensively before.

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 4, 2016

@ksooo do you mind trying again? It never crashed/froze for me. If it does freeze, can you check with gdb where it does it?

@ksooo
Copy link
Member

ksooo commented Jan 4, 2016

Not sure whether it is actually worth testing against 4.0.x before stuff got backported.

@perexg
Copy link
Contributor Author

perexg commented Jan 4, 2016

Just a note: TVH v4.1-1299-g80a50ce seems to catch all major timeshift bugs.

@ksooo
Copy link
Member

ksooo commented Jan 4, 2016

Just a note: TVH v4.1-1299-g80a50ce seems to catch all major timeshift bugs.

In combination with the pvr.hts fixes of this PR, I guess. Which Kodi version are you using for testing? Latest Jarvis beta?

@perexg
Copy link
Contributor Author

perexg commented Jan 4, 2016

No, 15.2 and code from PR #144 . But it should be fine to use also this cherry-picked code from this PR. All changes just throws 'obsoleted' packets received between request / response states. And, the wait call in CHTSPDemuxer::Seek was really broken (missing variable initialization).

@perexg
Copy link
Contributor Author

perexg commented Jan 4, 2016

Another "critical" timeshift bug fixed in v4.1-1301-g25b2cbc .

@perexg
Copy link
Contributor Author

perexg commented Jan 4, 2016

Cross-linking TVH bug (Jarvis testers): https://tvheadend.org/issues/2744

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 7, 2016

@perexg can you rebase?

@CvH
Copy link

CvH commented Jan 26, 2016

Any news on this ? @perexg

It's really a fatal error. If the predicate is not set to zero,
then _PredicateCallbackDefault() function (see platform/mutex.h)
gives true return value. It means that Wait() returns ASAP.

This also means that m_seekTime must not be set to a zero value
in the ParseSubscriptionSkip().
@perexg
Copy link
Contributor Author

perexg commented Jan 26, 2016

OK, rebased..

Jalle19 added a commit that referenced this pull request Jan 28, 2016
@Jalle19 Jalle19 merged commit 0b7c65c into kodi-pvr:master Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants