dbrock / bongo

Play music with GNU Emacs
http://www.emacswiki.org/emacs/Bongo
Other
207 stars 22 forks source link

VLC backend never fetches timing data #25

Closed iqbalansari closed 8 years ago

iqbalansari commented 8 years ago

It seems that the code that triggers bongo-vlc-player-start-timer in bongo-vlc-process-filter is never run, most probably because the patterns it looks for running bongo-vlc-player-start-timer are not emitted by the backend vlc process anymore

ghost commented 8 years ago

I'm also seeing this behavior with (VLC 2.2.0-rc2, from Debian stable). It looks like the status command should be periodically sent to poll the current state with the new RC, as it does not automatically print it.

State is also now indicated with a name instead of an integer, e.g

( new input: file:///home/shw/Music/Little Boots - Hands - 04 - Click.mp3 )
( audio volume: 38 )
( state paused )

New RC format. Old RC format. The switch appeared to be somewhere in March, possibly before.

Until the new RC format is supported, customizing bongo-vlc-extra-arguments to include -I oldrc works for me after manually seeking. I'll submit a PR to suggest it as the default and fix it so that it does not require the manual seek.

iqbalansari commented 8 years ago

@thwg great work figuring this out. I just tried your patch and it seems to work well, I will try it on an older VLC version today and report back

ghost commented 8 years ago

Awesome, will look forward to the details.

After testing a bit more, I believe the placement of this line (before the print) is the culprit causing the requirement of performing an action before getting the state information and starting bongo-vlc-player-start-timer.

It requires a single command to be read before initially sending the state information. Sending a blank newline after startup seems to work, and doesn't require the "audio volume" hack used in my PR (which is always printed at startup). It might be a cleaner approach.

As for detecting different VLC versions, if necessary, perhaps running a separate process beforehand (vlc --version) and determining whether to use rc or oldrc based on the printed version would be a good approach?

I'll have some time on Sunday evening to work on this a bit more.

iqbalansari commented 8 years ago

Looking at NEWS it seems that oldrc was introduced in version 1.1.0 (which was five years ago). Don't know about other distros but 12.04 is the oldest supported version for Ubuntu and it ships with VLC v2.0.8, as such I think VLC version check might not be required.

iqbalansari commented 8 years ago

BTW it worked well on v2.0.8 of VLC.

Sending a blank newline after startup seems to work, and doesn't require the "audio volume" hack used in my PR (which is always printed at startup). It might be a cleaner approach.

It would be great if it works without user interaction.

ghost commented 8 years ago

Thanks for the info; looks like it has been around longer than I thought! I agree, there probably isn't any need to differentiate between versions.

Here is a different patch which relies on the output of the VLC initialization message. I think it's a better patch seeing as that header is consistently there and only runs once, whereas I'm not sure the "audio volume" necessarily has to be printed. It should work at least through 2.0.8 (Ubuntu 12.04) and 2.2.1 (Ubuntu 15.10) -- roughly tested the output using Docker images of those distros.

It involves moving the timer start out of the play case, but seeing as --play-and-exit was added to the command in 2009, that may be a vestigial case -- I don't think that it should affect anything negatively by moving it outside.

Let me know if it also works well for you, and if so, I'll propose this instead of the previous PR. Thanks!

iqbalansari commented 8 years ago

Just tested it and seems to work well, great work :+1: , thanks!

dbrock commented 8 years ago

Great work researching this. Thanks a lot!