IPlayer: change GetTotalTime() to return milliseconds in an int64_t #1263

Merged
merged 1 commit into from Aug 10, 2012

Projects

None yet

4 participants

@Montellese
Member

This changes the definition (and usage) of IPlayer::GetTotalTime() from returning seconds in an int to returning milliseconds in an int64_t. Up until now there was no documentation and no rule for it and DVDPlayer returned seconds while the new PAPlayer implementation returns milliseconds (which is how I noticed it).

After a short discussion with @cptspiff I decided to adjust the definition of IPlayer::GetTotalTime() to be the same as IPlayer::GetTime() which returns milliseconds in an int64_t.

I hope I caught all the usages of the calls to GetTotalTime() of the different IPlayer implementations.

@arnova
Member
arnova commented Aug 8, 2012

What's the point of having ms resolution for these functions? I would favor having all functions using seconds as dimension.. And now CApp GetTotalTime() uses seconds and IPlayer uses mseconds, which seems confusiong...

@Montellese
Member

I simply adjusted it the way it works with GetTime(). IPlayer::GetTime() returns milliseconds and CApplication::GetTime() returns fractional seconds. As GetTime() was not broken but GetTotalTime was, that's the one I fixed. @cptspiff said on IRC that milliseconds resolution would be preferred over seconds resolution.

@ghost
ghost commented Aug 8, 2012

divide by 1000. done. other way is not doable. it does not hurt and may be useful. that is my line of thought.am I missing something?

@Montellese
Member

Agreed.

@elupus
Member
elupus commented Aug 8, 2012

Fully agree. Annoyed me for a long time. Alternatively they could be
doubles.

@arnova
Member
arnova commented Aug 8, 2012

I do like to recommend to properly document this in CApp.h for GetTime/GetTotalTime etc. And make clear in CApp:GetTime & GetTotalTime etc. that we have convert from ms to seconds to make it more obvious in the future..

@Montellese
Member

It already is documented but only in Application.cpp and not in Application.h. It says

Returns the total time in seconds of the current media. Fractional portions of a second are possible - but not necessarily supported by the player class. This returns a double to be consistent with GetTime() and SeekTime().

@Montellese
Member

I added a doxy description and a warning that these methods in CApplication and IPlayer do not return the same values to Application.h. Everyone satisfied now?

@huceke
Collaborator
huceke commented Aug 9, 2012

Don't forget the brand new android AMPLayer ;)

@Montellese
Member

@huceke Thanks for the reminder. Updated AMLPlayer as well.

@arnova
Member
arnova commented Aug 9, 2012

Thanks (for the effort) :-)

@Montellese Montellese was assigned Aug 9, 2012
@Montellese Montellese merged commit 841caf2 into xbmc:master Aug 10, 2012
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Jan 7, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
0d22ebc
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Jan 7, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
b810843
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Jan 12, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
60f4ada
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Jan 21, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
39e291e
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Feb 9, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
0cb59bc
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Feb 27, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
8f31acd
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Mar 21, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
3f8862d
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Mar 23, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
875753d
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Mar 23, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
a3894fb
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Mar 25, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
e684d40
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Mar 25, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
2be1fdc
@un1versal un1versal pushed a commit to un1versal/kodi that referenced this pull request Apr 1, 2015
uNiversaI [UPnPServer] xbmc -> kodi sender/CAnnouncementManager
not sure about a few xbmc -> kodi like line #107 #1261 #1263 #1265
24d54f5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment