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 improvements #19

Merged
merged 3 commits into from Mar 30, 2014
Merged

Conversation

Jalle19
Copy link
Collaborator

@Jalle19 Jalle19 commented Mar 27, 2014

Please read the individual commit messages for the details (not just the first line).

A few notes:

  • STimeshiftStatus uses a bool instead of an int since this is not C from 1995
  • It seems no one else implements PauseStream(), though I don't see why not
  • The SendSpeed() method is a bit funny since it assumes the speed parameter is in XBMC units, not tvheadend units. I think overall it would be best to factor out such differences to client.cpp so sanity and readability can be preserved in the inner layers (like I've done with GetPlayingTime()).

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Mar 27, 2014

Now that I've looked at it closer I think the last observation was an illusion. XBMC doesn't AFAICT display the live position compared to the shifted position in the progres bar. But at least it fixes the "time always goes forward even when skipping backwards" bug.

@adamsutton
Copy link
Owner

The pausestream function isn't required, we use setspeed instead.

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Mar 27, 2014

Wouldn't it be best to use both? From what I've gathered it can sometimes take many seconds before XBMC calls SetSpeed, possibly due to some internal buffering or something.

@adamsutton
Copy link
Owner

XBMC shouldn't use the pause routine because the add-on doesn't work like that. And yes, after speaking to Lars, the buffering is internal and is just the way it is.

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Mar 27, 2014

I dropped some of the commits, hopefully Github will realize it and update this PR. I tried to track down the root cause of the timeshift skipping bug but to no avail, all I managed to figure out was that it's XBMC's fault.

What it means is we convert tvheadend's notion of "seconds shifted" to an
absolute timestamp.

This fixes two issues. The first one is that if you pause a live stream at e.g.
10:00 program time (ie. 10 minutes after it has started), wait for 30 seconds,
unpause and then rewind 30 seconds, XBMC would display the current time as
11:00 when in fact it should be 10:00 again (since we rewinded the buffer to
the beginning).

The other issue is more of a new feature. Now when you pause a live stream
long enough XBMC will display the shifted position along with the live position
in the seek bar, so you know roughly where in the shift you are.
In fact we got a response but it was invalid or had no meaning (I'm not
totally sure what a time of -1 means)
@Jalle19
Copy link
Collaborator Author

Jalle19 commented Mar 27, 2014

Something went wrong when I forced push, now it should be up-to-date.

@ghost
Copy link

ghost commented Mar 29, 2014

Good job Jalle19!
There is one use case that is missing, when we reach either end of the stream we probably need to send 0 to xbmc.
If I rolled all the way back to the beginning of the stream buffer and I hit step back again, the display time is reduced again, which is not correct.

@ghost
Copy link

ghost commented Mar 29, 2014

... strange I couldn't repro when I start with a new stream, only happened if I step back through a ~15 minutes buffer, I wonder if this has to do with the dvdplayer loop bug, cause after I hit the end of the stream I started experiencing the timeshift loop issue.

@Jalle19
Copy link
Collaborator Author

Jalle19 commented Mar 29, 2014

@Danilll I think the remaining issues (after your overflow patch) lie with XBMC.

@adamsutton adamsutton merged commit adee675 into adamsutton:master Mar 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants