Skip to content

fixed problem with streams that update display time (DVD,BluRay), fixes smallstepback #1621

Closed
wants to merge 1 commit into from

3 participants

@dragonflight

Time is updated from stream (in the case of DVD and Bluray) when read as opposed to when displayed so they are aprox 7/8 secs fast (because of the cache)

patch tags the packets with the stream time and the player updates its idea of time when it displays a packet.

@dragonflight dragonflight fixed problem with streams that update time (DVD,BluRay)
tagged packets with stream time at reading
update stream Current time when playing
755138c
@dragonflight

I have only changed the Video player as I believe that is sufficient, to be absolutely correct one could update the audio player as well anf display the most recent time. If people think that is the right way I can make the changes.

@elupus elupus was assigned Oct 14, 2012
@elupus elupus commented on the diff Oct 19, 2012
xbmc/cores/dvdplayer/DVDPlayer.cpp
if (m_CurrentVideo.dts != DVD_NOPTS_VALUE)
state.dts = m_CurrentVideo.dts;
else if(m_CurrentAudio.dts != DVD_NOPTS_VALUE)
state.dts = m_CurrentAudio.dts;
+*/
else
@elupus
Team Kodi member
elupus added a note Oct 19, 2012

This is wrong. the dts here is used for other things

@dragonflight
dragonflight added a note Oct 19, 2012

As far as I could tell the dts is only used for calculating state.time_offset, which it only does if it is a DVDSTREAM_TYPE_DVD (which I think is wrong as one needs the same adjustment for Blurays (?). In any case it is a diff with state.time which is now (would be) on "display time" not "read time" - but I wasn't sure which is why I left the original as a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elupus
Team Kodi member
elupus commented Oct 19, 2012

While the idea is right. I don't like the implementation.

I think this should be a separate message pushed down the video queue if video is in charge of time and audio if audio is in charge. Then the player should timestamp the message and re-post that back up into parent message queue which update current playback time.

@dragonflight

I'm not sure I understand (well actually I am sure I don't).
If the stream decoder is responsible for determining "real time", then it should tag the display packet with that information, as it needs to be synchronized with that packet.
As for understanding the current "real-time" I agree that it should have been the responsibility of all players to update time and a message to update time would be a good choice.
I just wanted to fix a very annoying bug (not sure why others didn't complain - the queue is just the same length as as small backstep, making it just a stutter:)) and I thought poking into the player was in the same vein as poking into the decoder.
perhaps you could comment more on what you mean with the messages

@MartijnKaijser
Team Kodi member

Is this PR still needed for current master?

@MartijnKaijser
Team Kodi member

@dragonflight
see if this is still needed with current master. if so please update and ping elupus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.