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

VideoPlayer: set demuxer speed other than pause and normal #14488

Merged
merged 1 commit into from Feb 28, 2019

Conversation

Projects
None yet
3 participants
@FernetMenta
Copy link
Member

commented Sep 28, 2018

@ksooo

This comment has been minimized.

Copy link
Member

commented Oct 3, 2018

Tested with pvr.hts plus tvheadend. Unfortunately, this change leads to (green) picture artifacts while seeking. So, no improvement. Things got even worse than before.

@FernetMenta

This comment has been minimized.

Copy link
Member Author

commented Oct 3, 2018

this change leads to (green) picture artifacts while seeking

I doubt. Seek does not change playspeed. Means this code won't get hit on seek.

@ksooo

This comment has been minimized.

Copy link
Member

commented Oct 3, 2018

Oops, I used wrong words. I was actually writing about fast forward / rewind.

@FernetMenta

This comment has been minimized.

Copy link
Member Author

commented Oct 4, 2018

This PR reinstates behaviour of v17. Current master only sets demuxer speed for normal and pause. If tvh can go faster than normal it does not need setting of demuxer speed and you could ignore the calls in the addon. Other pvr and inputstream addons need this, see trac ticket.

@ksooo

This comment has been minimized.

Copy link
Member

commented Oct 4, 2018

Before this PR, it worked better. Please do not let "suffer" official add-ons in favor to some random unofficial add-on (referring to the trac ticket, where the submitter not even reveals details of his add-on) - at least not in beta stage. I guess I will not find the time soon to adjust pvr.hts. Thanks.

@FernetMenta

This comment has been minimized.

Copy link
Member Author

commented Oct 4, 2018

For the record: this reinstates v17 behaviour. The issue is clearly tvh.
If you block this PR you do this in favour of malfunctioning tvh and against correct behaviour and other addons you may not be aware of.

The comment in this code give a clear and valid reason why this change is required. On the other hand we don't know a valid reason why tvh addon abuses this function.

I leave the decision to you and the team.

@ksooo

This comment has been minimized.

Copy link
Member

commented Oct 4, 2018

Sorry, not looking at the code but on the screen as a user, v17 does not show the artifacts while ff/rew. So, from user's POV this PR introduces a regression for one of our most used pvr add-ons. Not good.

I'm not saying that there is nothing to improve/fix in pvr.hts / tvheadend. I'm only saying that this is not a good timing (we're in beta) for this change and that I'm afraid I will not have time to adjust pvr.hts and I don't have the skills to fix tvheadend if it turns out to be the root cause.

@FernetMenta

This comment has been minimized.

Copy link
Member Author

commented Oct 4, 2018

Ignoring speed changes other than pause and normal in tvh addon is adding 2 lines

https://github.com/kodi-pvr/pvr.hts/blob/b244c01513e6671751058316406f68123b812cef/src/tvheadend/HTSPDemuxer.cpp#L199

if (speed != 0 && speed != 1000)
return;
@ksooo

This comment has been minimized.

Copy link
Member

commented Oct 4, 2018

Ignoring speed changes other than pause and normal in tvh addon is adding 2 lines

I have other priorities at the moment. I will put this on my list, but do not make any promises when it will be ready.

@ksooo ksooo merged commit 363b794 into xbmc:master Feb 28, 2019

1 check passed

default You're awesome. Have a cookie
Details

@Rechi Rechi added this to the Leia 18.2-rc1 milestone Feb 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.