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

Changed Player from VLC to MPV or MPG123 #50

Closed
wants to merge 13 commits into from
Closed

Changed Player from VLC to MPV or MPG123 #50

wants to merge 13 commits into from

Conversation

Luegengladiator
Copy link
Contributor

Splitted the scripts to use Settings directory and created dedicated scripts for VLC, MPV and MPG123.
To send keystrokes async to MPG123 I used Terminal Multiplexer TMUX.
Additional I renamed the running process of VLC/MPV/MPG123 with exec -a RPi-Jukebox-Player to identify the running process without knowing the underlaying player.
So it's possible to get the correct running Process and kill/stop it.
With the PID of the RPi-Jukebox it's now possible to get the Playlist from /proc and stop the running Playback on second read of the same RFID.
MPG123 has a Problem storing the Position in the Track. So I used Conplay from the mpg123 project
https://github.com/nadult/mpg123/blob/master/scripts/conplay
So Position of the Track is saved inside the m3u.
Creating the m3u always new is now a Problem so I fixed that too. Create only if not existing.

Actual missing: Possibility to remove the m3u or at least the comment inside to restart the Playlist from beginning

@MiczFlor
Copy link
Owner

HI @Luegengladiator
wow. There is a lot of stuff in here to check and add. You had planned to add MPV in order to fix some issue, I believe to pick up longer playlists where you left them off and not start from scratch?
The necessity to do playlists only if they don't exist is a conflict with the new feature to play podcasts which - by their nature - need to be newly created when you swipe the card. I guess I will need to add this in the script.
What is the benefit of adding MPG123 in your eyes?
Thanks for the contribution, it will grant me many sleepless nights of testing and conflict resolution, I am sure :)

@Luegengladiator
Copy link
Contributor Author

I started with MPV and it was up and running with a socket in the background (MPG_playout_controls and rfid_trigger_play). But there is a 8 Second startup delay on RPi 1.
I'll use the Phonie as a Musicplayer for my little son and he is impatient so 10 sec is to long.
So I was looking for a lightweight background commandline musicplayer storing Positions. mpg123 is very lightweight, used in you System already, but not able to store position. That was fixed by nadult with the conplay (continous play) script. Background is fixed with tmux so mgp123 was the best solution with a startupdelay <1 sec.

MPG123 is a fast console mpeg audio decoder/player. Build for audio only. The Phonie is not designed to play Videos so it's not necessary to use a heavyweight Videoplayer like VLC or MPV.

@MiczFlor
Copy link
Owner

MiczFlor commented May 30, 2018

Ok, got it. And a very elegant solution to use MPG123. Good work.

Plus: I totally understand the impatience issue :) my daughter needed the stop chip tied on a string to the box to stop when she wants, immediately, otherwise she unplugged the power supply - which cost me a few SD cards :)

I remember now you once asked if there could be an event triggered when removing the RFID chip. So this is the idea: you swipe the card a second time and the audio will stop / store the current position in a file and/or playlist, swipe another time: the playout continues exactly where it started.

Without having looked at the code in detail (yet):

  • Did you also implement the pause / continue with VLC?
  • Does your solution for MPG123 also work after a shutdown?

And: could you take a look at this and give me some feedback? (I will ping @Bockiii here as well, because I think there is an overlap in thinking)
#31

The pull requests are related, I haven't tested this for VLC yet either. Not sure if this seeks the position within a file, too. Or just jumps to the last file in the playlist.

I will need to do some conflict resolution and do some cherry picking to your pull request(s).

Great work, thank you for the contribution.

@Luegengladiator
Copy link
Contributor Author

Luegengladiator commented May 30, 2018

At the moment I focused on mpg123 because all parts are there.
I didn't touch VLC and MPV got only the default running parameters.

The Scripts checks on RFID-trigger. If the RFID corresponds to the running process it send a "stop" command. If it's a different one, it stops the actual one, storing the position and starts the new one.
So the "remove"-Task is not needed any more.

Conplay writes a comment to the m3u like
#M3U
#current entry: 25
#current frame: 4765
so the continue works for conplay on that file as long as the comment is present.
I'm looking for a solution to restart the listing. Something like 1) read a special RFID 2) wait for the rfid to clean up 3) drop/clean the m3u.
But thats a different project.

I did not check the mpg123 on podcasts but got NDR2 (local radio broadcast here) livestream running with it.

@Luegengladiator
Copy link
Contributor Author

Luegengladiator commented May 30, 2018

Play/Pause/Continue seems to be a bigger thing. #47 and #31 are working on it. And my first approach with MPV was looking for a solution too.
I had a first look at #31 . The Idea there is to write a dedicated track-file. So this is not a big Problem. Due to the different playout-controls you can decide to change comments inside m3u or inside .track.

I think only the User can decide which player is the best for the things he wants to do.
So it's a good Idea to support different Player. Maybe with a decision Matrix inside the Documentation?

@MiczFlor MiczFlor changed the base branch from master to pr50-testing-changeplayer June 22, 2018 10:12
@Luegengladiator
Copy link
Contributor Author

I'm working on the "podcast-Problem" planing to name the Audiofolder like A_PutYourNameHere for Audiobooks, M_PutYourNameHere for Musik, Y_PutYourNameHere for youtube, P for Podcast ....
So it's a user decision where to store the playposition and where not.
Finished the playcontrol "restart track" and "restart list".

@MiczFlor Is it possible to update the PullRequest or should I wait for the finished merge?

@MiczFlor
Copy link
Owner

MiczFlor commented Jul 9, 2018

Hi @Luegengladiator

thanks for getting in touch about this, I was thinking about that, too. Right now I have too little time to really get my hands dirty with a task to implement your solution, but I am very happy to be your "rubber duck" for the task (see: https://en.wikipedia.org/wiki/Rubber_duck_debugging )

As the maintainer, what I would like to see ideally is that you approach this in stages, each of which becomes a pull request. (and that would mean that I would like to close this pull request and invite you to start creating a new one based on the latest code base, because there are so many conflicts by now...).

Breaking this down into smaller steps for a roadmap will make it easier for others to chip in as well and help to debug each new feature.

Regarding the podcast problem (as you describe it), I am also thinking about how to handle this best to have a minimum impact on the users. The more complicated the software gets, the less people will use it and contribute to it. It might be best if we have a brief chat about this? You seem to be based in the German language area, based on your username, so please mail me micz.flor KLAMMERAFFE web.de and we can arrange a call?

The stages I want to suggest would be:

1: Adding MPG123 as alternative player

This as an option in settings would be great to have. I like the solution and I assume it will be battery saving and responding faster. For now, I would drop MPV altogether, because so much work has gone into VLC by now and the benefit of MPV is not visible to me anymore, given this: #47
Step one: just add this option.

2: Adding 'resume play' for MPG123

While you might think this could be all in one, I think it should be a separate step, because it adds a separate script. This would be the solution you already have done: #50 (your description at the top)

3: Adding 'resume play' for VLC

This is summarized here: #47
This is something, I might then be adding and you could focus on:

4: Support of EAN Scanner

#65
Which I think is a great idea. And it will require us to coordinate well how this is being activated and/or deactivated in the settings. For the time being, I believe the two could live happily active side by side. But we talk about that, when the time comes

The apt-get install phoniebox is for the future, I think features are the present. What do you think?

I suggest we find a time to talk soon and spread tasks between us, and discuss approaches to make it feature rich and dumb to use.

All the best, micz

@Luegengladiator
Copy link
Contributor Author

agreed. Let's line up and coordinate the merge on an actual Version of the Jukebox

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.

2 participants