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

Pushed subtitle offset default from +-10s to +-60s #4929

Merged
merged 1 commit into from Aug 2, 2014

Conversation

topfs2
Copy link
Contributor

@topfs2 topfs2 commented Jun 17, 2014

While this is perfectly doable for anyone via advancedsettings.xml I think it might be valid to make the default a bit larger than 10s. What I've seen when used offset is that either its syncing alright and then +-10s is fine to make it ok for the entire video, or its drifting (not sure why it drifts but) and then 10s is far from enough.

@arnova
Copy link
Member

arnova commented Jun 18, 2014

Makes sense. Although rare, I've ran into this myself a couple of times in the past.

@t-nelson
Copy link
Contributor

I haven't looked at the affected code. Does this reduce resolution or just take forever to get to the ends?

@arnova
Copy link
Member

arnova commented Jun 18, 2014

I believe mainly the latter since we don't increase speed on eg. button hold.

@t-nelson
Copy link
Contributor

Right so the step is fixed rather than being max/100 or similar. Usability
should be considered in this case. I suspect the actual slider is of no
value as a visualization tool as the steps will likely be less than one
pixel.

@Jalle19
Copy link
Member

Jalle19 commented Jun 18, 2014

This usually happens when the subtitles and the video you're watching have commercials cut differently. This tends to introduce many seconds of desync, and after a few commercials 10 seconds is not enough to compensate.

@topfs2
Copy link
Contributor Author

topfs2 commented Jun 19, 2014

Yup, that is probably the main reasons behind my desync.

@t-nelson Its true that the usability gets a bit different here, its harder to notice if the slider goes left or right, however with keys that should be a no problem since you know if you pressed left or right. On touch/mouse devices the resolution gets trickier, I haven't tried it there.

But from a usability standpoint the slider is not ideal in this, what happens with the sub when you go left is not obvious, I know that I generally just try both and see what makes it better :P Combine this with that ideally we probably wouldn't even want a roof of how much you alter the sync. Perhaps a more long time solution would be to reinvent the popup entirely which solves both usability and UX.

IMO however this is a good temporary solution, atleast on stb

@piejanssens
Copy link

+1

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Jul 16, 2014
@topfs2
Copy link
Contributor Author

topfs2 commented Jul 20, 2014

Ok if I pull this next window?

@Jalle19
Copy link
Member

Jalle19 commented Jul 20, 2014

Fine by me

@topfs2
Copy link
Contributor Author

topfs2 commented Aug 2, 2014

jenkins build this please

topfs2 added a commit that referenced this pull request Aug 2, 2014
Pushed subtitle offset default from +-10s to +-60s
@topfs2 topfs2 merged commit 6f26fca into xbmc:master Aug 2, 2014
@MartijnKaijser MartijnKaijser modified the milestones: Pending for inclusion, Gotham13.x Aug 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants