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

VLC version 3.0.0 uses times in microseconds #109

Merged
merged 1 commit into from Aug 14, 2016

Conversation

Projects
None yet
1 participant
@ghost

ghost commented Aug 9, 2016

Hi,

this pull request fixes the Syncplay plugin for VLC 3.0.0.

@Et0h

This comment has been minimized.

Show comment
Hide comment
@Et0h

Et0h Aug 9, 2016

Contributor

Looks pretty elegant. Why does the multiplication/division also apply to the title check - is the title value also off by such a large factor?

Contributor

Et0h commented Aug 9, 2016

Looks pretty elegant. Why does the multiplication/division also apply to the title check - is the title value also off by such a large factor?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 10, 2016

Hi,

yes you are correct. I didn't try with DVD movie yesterday. I've updated the pull request accordingly.

ghost commented Aug 10, 2016

Hi,

yes you are correct. I didn't try with DVD movie yesterday. I've updated the pull request accordingly.

@Et0h

This comment has been minimized.

Show comment
Hide comment
@Et0h

Et0h Aug 14, 2016

Contributor

I tried testing it and it appears setting the position to >= 2148 seconds results in VLC setting the position to 0. Do other people have this problem, and is this a VLC 3.0 bug?

Contributor

Et0h commented Aug 14, 2016

I tried testing it and it appears setting the position to >= 2148 seconds results in VLC setting the position to 0. Do other people have this problem, and is this a VLC 3.0 bug?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 14, 2016

I have the same problem with this.

I could not find the 'time' variable initialisation in vlc, but if it is set to int, the boundaries check will reset it to it's minimum if it's under it (vlc/src/misc/variables.c 192->240). Since the int can contain numbers up to +2147483647 which in our case is 2147.483647 seconds, when provided with a number 2148000000 it will overflow and will become negative. The mentioned check will see that it is under the minimum allowed value for this variable which is 0 seconds and will automatically set it to 0.

I assume that when they changed seconds to microseconds they forgot the change the variable time to float, which can handle bigger numbers.

ghost commented Aug 14, 2016

I have the same problem with this.

I could not find the 'time' variable initialisation in vlc, but if it is set to int, the boundaries check will reset it to it's minimum if it's under it (vlc/src/misc/variables.c 192->240). Since the int can contain numbers up to +2147483647 which in our case is 2147.483647 seconds, when provided with a number 2148000000 it will overflow and will become negative. The mentioned check will see that it is under the minimum allowed value for this variable which is 0 seconds and will automatically set it to 0.

I assume that when they changed seconds to microseconds they forgot the change the variable time to float, which can handle bigger numbers.

@Et0h

This comment has been minimized.

Show comment
Hide comment
@Et0h

Et0h Aug 14, 2016

Contributor

Thanks for the investigation. I've created a ticket at https://trac.videolan.org/vlc/ticket/17285#ticket

Contributor

Et0h commented Aug 14, 2016

Thanks for the investigation. I've created a ticket at https://trac.videolan.org/vlc/ticket/17285#ticket

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Aug 14, 2016

You probably meant vlc.var.set(vlc.object.input(), "time", 2148000000) as example for the ticket?

ghost commented Aug 14, 2016

You probably meant vlc.var.set(vlc.object.input(), "time", 2148000000) as example for the ticket?

@Et0h

This comment has been minimized.

Show comment
Hide comment
@Et0h

Et0h Aug 14, 2016

Contributor

Yup, that's the one 👍

Contributor

Et0h commented Aug 14, 2016

Yup, that's the one 👍

@Et0h Et0h merged commit 9edfb8e into Syncplay:master Aug 14, 2016

@ghost ghost deleted the vlc3.0.0_fix branch Aug 15, 2016

@Et0h

This comment has been minimized.

Show comment
Hide comment
@Et0h

Et0h Aug 30, 2016

Contributor

351024f means Syncplay now fails with an appropriate error message when you try to use Syncplay with VLC 3 due to its lack of Syncplay support. Anyone wishing to fix the microsecond issue can see https://trac.videolan.org/vlc/ticket/17285 - either VLC Lua needs to support the values beyond microseconds, or needs to use the old decimal seconds system for the Lua "time" variable. Discussion on this topic should take place within #108

Contributor

Et0h commented Aug 30, 2016

351024f means Syncplay now fails with an appropriate error message when you try to use Syncplay with VLC 3 due to its lack of Syncplay support. Anyone wishing to fix the microsecond issue can see https://trac.videolan.org/vlc/ticket/17285 - either VLC Lua needs to support the values beyond microseconds, or needs to use the old decimal seconds system for the Lua "time" variable. Discussion on this topic should take place within #108

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment