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

VideoReferenceClock: refactor, fixes, video sync for RPi #5582

Merged
merged 6 commits into from
Nov 1, 2014

Conversation

FernetMenta
Copy link
Contributor

The old VideoReferenceClock has become a big mess and combined all platform specific code into a single file. So any fix or addition required a refactoring first. I stated with it because I wanted to get rid of the brittle GLX implementation for Intel systems. The invisible window with its GLX context often caused trouble.

In parallel @popcornmix worked on a clock for the Pi which is now included here.

credits to @Memphiz who did the work for OSX/iOS. In order not to break future bisects I squashed your changes into the refactor commit.

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@wsnipex
Copy link
Member

wsnipex commented Oct 27, 2014

the linux build error is caused by missing libdrm in unified deps.
you'll have to add /usr/lib/x86_64-linux-gnu/pkgconfig/libdrm.pc to https://github.com/xbmc/xbmc/blob/master/tools/depends/target/Makefile#L128

We might also need to install libdrm-dev on the jenkins slave, didn't check that yet.

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@MartijnKaijser
Copy link
Member

@topfs2

@fritsch
Copy link
Member

fritsch commented Oct 30, 2014

While this is running perfectly fine on my hardware (intel), I think this could also work nicely on AMD OSS hardware - can anyone test this? (I need to reinstall my amd boxes before getting that running).

Edit: This line needs adjusting: FernetMenta@9012809#diff-cd6e9ebd37bcd0088ec3f06fc3054fdeR100 to test it on AMD

@wsnipex
Copy link
Member

wsnipex commented Oct 30, 2014

from a quick test on my kabini this seems to be working fine on AMD as well

@FernetMenta
Copy link
Contributor Author

Thanks for testing but I won't enable it for AMD in this PR. It needs much more testing on various hw. @fritsch if you feel comfortable with this change for AMD had have more data from testing, feel free to submit this as a separate PR later.

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@wsnipex
Copy link
Member

wsnipex commented Oct 30, 2014

I've watched >1h of IPTV news streams and didn't notice any issues, even when using 3:2 pulldown(50Hz stream on 60Hz in windowed mode).

@FernetMenta how about enabling this in your master for some enduser testing?

@FernetMenta
Copy link
Contributor Author

@wsnipex good idea. will do

@fritsch
Copy link
Member

fritsch commented Oct 30, 2014

@wsnipex @FernetMenta This should enable it on Intel and AMD: fritsch@65f08d5

@FernetMenta
Copy link
Contributor Author

@fritsch fixed a compiler warning, thx
jenkins build this please

@MartijnKaijser MartijnKaijser added this to the Helix 14.0-alpha5 milestone Oct 31, 2014
@MartijnKaijser
Copy link
Member

it was approved and jenkins builds ok

FernetMenta added a commit that referenced this pull request Nov 1, 2014
VideoReferenceClock: refactor, fixes, video sync for RPi
@FernetMenta FernetMenta merged commit ba267af into xbmc:master Nov 1, 2014
@FernetMenta FernetMenta deleted the refclock branch November 1, 2014 06:02
@Voyager1
Copy link

Voyager1 commented Nov 9, 2014

something's wrong with the refactor for Windows (D3D). Sync playback to display at 23.976fps is extremely stuttering (getting around 14-15fps).
I noticed that when pressing "o" the refresh is still listed as 50fps (although the monitor has switched to 24fps). That could cause the issue.
I reverted this whole PR and the problem is solved.
When pressing "o", the refresh rate is now correctly listed as 24fps.
I'm going to take a closer look at the potential cause, but I guess it's a refactoring mistake in one of the VideoSync classes.

Note: my desktop res refresh is at 50fps - and auto refresh change is enabled. I have the impression that the new GetFPS logic doesn't detect the change - as was previously signaled by the graphicsContext.

@fritsch
Copy link
Member

fritsch commented Nov 9, 2014

Does this help: #5679 ?

@Voyager1
Copy link

yes thanks that solved the issue.

@Memphiz
Copy link
Member

Memphiz commented Nov 10, 2014

@FernetMenta mhhh using beta2 on my atv2 now (which is a release build!) and when watching freetv (60hz refreshrate on tv - 25fps in the stream) i get frame drops maybe one a sec or so - not each sec but really constantly. The strange thing is - as soon as i enable debug logging it works flawless again (seeing the onscreen debug text and it lasts some time and then gets to a rock solid 25fps).

You have any idea how this could happen? (no debugger on atv2 - and without debug log its kinda hard to tell anything i guess?)

@FernetMenta
Copy link
Contributor Author

you resample audio right? (otherwise this setting don't make any sense) what resample ration does the codec screen show? is it constant?

@fritsch
Copy link
Member

fritsch commented Nov 10, 2014

Having him on IRC. He has Sync Playback to Display disabled - so most likely not this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants