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

delete the mpv tmp file after creation #266

Closed
wants to merge 1 commit into from
Closed

Conversation

lucat1
Copy link

@lucat1 lucat1 commented Dec 8, 2022

Previously, a temporary path was not only being generated but also touched, therefore the os.Stat check to verify mpv had started was really doing nothing and just succeeding at the first attempt.

Closes #265

In order to test on your machine (a Pi is not needed I believe) you can just add an mpv sh script to your $PATH (before the actual mpv) with just a sleep 10 inside. This will simulate a somewhat slow process starting (what I guess happens on a Pi). Without this change, the error would have been failed to connect as the startup check would always return a false positive. Now it properly hangs for 5s and errors out with mpv never started.

If you have the chance @marlop352, give this patch a try on your Pi. I don't currently have one on hand.

Previously, a temporary path was not only being generated but also
touched, therefore the os.Stat check to verify mpv had started was
really doing nothing and just succeeding at the first attempt.

Closes sentriz#265
@marlop352
Copy link

@lucat1 any instructions on how to do so using docker?
I would rather not install anything bare metal on the Pi unless I can't avoid it.

@sentriz sentriz closed this in e8fac56 Dec 8, 2022
@sentriz
Copy link
Owner

sentriz commented Dec 8, 2022

ah i see! thanks makes sense. i went with a slightly different version:

  • create a temp dir
  • set the sock path to tempdir/sock, but don't create it
  • let mpv create the sock file in the tmp dir
  • cleanup the tmp dir later

the reason being if we create a tmp file, delete it, create it again - there is a very very slim chance that is already exists. where as if we create a tmp dir then go will make sure there's no clash

i did some light testing and seems to work - but let me know if not

thanks!

@sentriz
Copy link
Owner

sentriz commented Dec 8, 2022

PS @marlop352 i just kicked off a job to build the nightly image here

so when it's done you can test out the change with the sentriz/gonic:nightly tag

sentriz added a commit that referenced this pull request Dec 8, 2022
@lucat1
Copy link
Author

lucat1 commented Dec 8, 2022

@sentriz that's totally fine. Glad it's fixed now

@marlop352
Copy link

PS @marlop352 i just kicked off a job to build the nightly image here

so when it's done you can test out the change with the sentriz/gonic:nightly tag

I tested the nightly image and it is working now.

I just need to figure out how to force it to use the correct audio output.

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.

mpv not starting on arm/raspberrypi
3 participants