Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[pvr] fixed - seek passed the time since the epg item started, which … 
…won't work for seek operations
  • Loading branch information
@opdenkamp
opdenkamp committed Dec 16, 2012
1 parent 99f082d commit c1f11e6
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion xbmc/pvr/addons/PVRClient.cpp
View file
Expand Up @@ -29,6 +29,7 @@
#include "settings/AdvancedSettings.h"
#include "utils/log.h"
#include "settings/GUISettings.h"
#include "Application.h"

using namespace std;
using namespace ADDON;
Expand Down Expand Up @@ -877,7 +878,10 @@ bool CPVRClient::SeekTime(int time, bool backwards, double *startpts)
{
if (IsPlaying())
{
try { return m_pStruct->SeekTime(time, backwards, startpts); }
// player time is added to time here, which is taken from the epg
// we can either substract it again here, or add special pvr cases in players
int iChangeTime = time - (int)g_application.m_pPlayer->GetTime();
try { return m_pStruct->SeekTime(iChangeTime, backwards, startpts); }
catch (exception &e) { LogException(e, "SeekTime()"); }
}
return false;
Expand Down

8 comments on commit c1f11e6

@FernetMenta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is wrong now after: 4c90033

subtraction happens in player.

@opdenkamp
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, i haven't tested this after that patch, but that's the place where it should be fixed indeed. but this will need to be synced with other players too that are using pvr.

@opdenkamp
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FernetMenta @adamsutton said seek times were still correct for him with pvr.hts after this (when there's an epg tag, without a tag, this will be wrong). note that this only affects add-ons that use SeekTime, so demux only

@FernetMenta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I don't see how this is related to epg times. seekTime is supposed to be dts. In dvdplayer:

state.time = actual time (for pvr)
state.time_offeset = DVD_MSEC_TO_TIME(state.time) - state.dts;

if(dynamic_castCDVDInputStream::ISeekTime*(m_pInputStream) == NULL)
time -= m_State.time_offset;

-> time = state.time - (state.time - state.dts) = dts

I have debugged this to death and I always had a valid epg tag.

@opdenkamp
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the epg start time was added to the player time iirc. i'll have to dig into it when i got time to provide a more detailed answer. with this commit in, the result (when epg info is available) is the relative seek time in millis, but only when an epg tag is present for the channel that's being played.

@adamsutton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm staying out of this: I've not followed the stuff going on in the xbmc cores nor do I care what units make it to pvr.hts, we can deal with that as required in the addon if its not exactly what TVH requires.

But I agree that I don't like PVRClient getting in the way, I had always intended, when I added the SeekTime() call to the API that it would simply pass the message straight through to the addon and let that handle it.

If there are special circumstances I'm not aware of then I'll accept that I'm, as usual, out of my depth in the XBMC code.

@FernetMenta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the epg start time was added to the player time iirc.

after @elupus commit it does not matter what state.time is. see equation above, the result is dts regardless of whatever value state.time has

I have observed incorrect seeks when using small steps like 5 secs. Target dts is calculated with reference to dts end of buffer. IMO this is wrong, it has to be calculated with reference to what the users sees on screen.
FernetMenta@fabf24e

Otherwise a small step back can result even in a step forward.

@elupus
Copy link
Contributor

@elupus elupus commented on c1f11e6 Jan 18, 2013 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.