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

Changes needed for Falcon Pi Player #417

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Materdaddy
Copy link

The MultiSync Video support required minor changes to the omxplayer binary.
These changes include:

  • Fixing a speed-adjustment bug
  • Adding new commands for fine-grained speed control necessary for syncing
    video running on multiple Pi units across a network.
  • Adding a "-V" verbose option to increase the status output frequency to
    occur more often to allow for tighter sync between multiple Pi's.

The MultiSync Video support required minor changes to the omxplayer binary.
These changes include:

- Fixing a speed-adjustment bug
- Adding new commands for fine-grained speed control necessary for syncing
  video running on multiple Pi units across a network.
- Adding a "-V" verbose option to increase the status output frequency to
  occur more often to allow for tighter sync between multiple Pi's.
@popcornmix
Copy link
Owner

I think this should really be using dbus. The extra status messages and the super-fine speed adjustments are of no use to omxplayer generally.

Querying state and setting speed through dbus sounds like the right solution.

@Materdaddy
Copy link
Author

Falcon Pi Player is an appliance-like stack to drive Christmas light displays. We want our image download to be as small and efficient as possible. Adding dbus to our system is a relatively major change and is likely more overhead and complication that's unnecessary. We were hoping to get these changes into mainline so we don't have to build omxplayer, but if that isn't a possibility, we'll probably just keep running with a fork. Thanks for the reply.

@popcornmix
Copy link
Owner

omxplayer already uses dbus by default. You can't control omxplayer without it. See:
https://github.com/popcornmix/omxplayer/blob/master/dbuscontrol.sh

for examples of how to query/control omxplayer.

@Materdaddy
Copy link
Author

Oh. I tried running "dbus" on the wrong image, not on my Pi. I'll look into this option.

@Materdaddy
Copy link
Author

After doing some quick research, would you be against merging in a pull request that adds the micro speed adjustments like in this current request so I can use dbus key commands similar to the examples for things like hiding video? I'd rather not code up a bunch of dbus changes in omxplayer to set speed adjustments through dbus.

I can pull out the parts of our changes that print the stats with higher frequency since we can use dbus to get the position in microseconds.

@jehutting
Copy link

@Kennyyy24 seems to have implemented the Rate method (but will be changed in the future to a property) in issue #276. Look at mpris2 interface MediaPlayer2.Player Rate
upon which kennyyy24 (in his repository mentioned in the issue, and is work in progress) calls SetSpeed.

@popcornmix
Copy link
Owner

I don't think we want keypress support for micro speed adjustments, as they are of no use to a normal user. The rate property dbus method sounds like the right solution.

@vcrocher
Copy link

Yes it's part of my plan to implement and integrate the Rate method through DBus (actually the Rate Property more than method). Still this is a quite tricky one given the current implementation of omxplayer and I don't have the best setup to test it at the moment so if anyone is interested and can do some testing please do so. In the current state it should be working, at the very least for reasonably low-speeds (not implemented the TRICK_PLAY yet).

To test it with a simple dbus send:
dbus-send --print-reply=double --session --reply-timeout=500 --dest=org.mpris.MediaPlayer2.omxplayer /org/mpris/MediaPlayer2 org.freedesktop.DBus.Properties.Set string:"org.mpris.MediaPlayer2.Player" string:"Rate" double:3.0

or whatever value between 0.01 and 10.25 .

@jehutting
Copy link

@Materdaddy just out of curiousity: is the syncing by using the (-m) stats values from the BUFFERED stdout of omxplayer accurate enough? Don't you get burst of omxplayer output due to the buffering?

@Materdaddy
Copy link
Author

@jehutting the values are plenty accurate. Our program typically loops at 50ms intervals while using select with a timeout reading from omxplayer. We adjust the speed based on a few factors. Our code is open source and you can take a look at what we're doing here: https://github.com/FalconChristmas/fpp/blob/master/src/mediaoutput/omxplayer.cpp#L285-L309

@jehutting
Copy link

@Materdaddy thanks for the info.
I am surprised too see that you have a 50 msec interval time. I would expect that omxplayer outputs in burst with intervals of over a couple of seconds (depending on the amount of output the 'mstat option' gives) due to linux buffered output.
Will have a look at your code. Thanks.

@Ruffio
Copy link

Ruffio commented May 4, 2016

@popcornmix, @jehutting is this something that could be implemented or?

@popcornmix
Copy link
Owner

I'm happy for the micro speed adjustments implemented through dbus, but not as this PR stands.

@Ruffio
Copy link

Ruffio commented May 4, 2016

@popcornmix what will it take to 'correct' this PR?

@Ruffio
Copy link

Ruffio commented May 17, 2016

@popcornmix what will it take to 'correct' this PR? Can you instruct @Materdaddy ?

1 similar comment
@Ruffio
Copy link

Ruffio commented Dec 29, 2016

@popcornmix what will it take to 'correct' this PR? Can you instruct @Materdaddy ?

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.

5 participants