Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Destination and /tmp must be on the same partition #39

Closed
spockout opened this Issue Sep 27, 2012 · 4 comments

Comments

Projects
None yet
4 participants

This probably doesn't effect too many people, but I bumped into it, so thought I'd share.

I'm running Linux Mint from a thumb drive. (Also Raspberry Pi with Raspbian.) I moved /tmp to tmpfs to minimize writes to flash. This breaks the creation of the ID3/MP4 tag when the song finishes because the rename() function requires that source and destination are on the same file-system. The error message is "Could not overwrite the audio file (18:Invalid cross-device link). Failed to write the tag."

This is also a problem if you want to have the files saved to a separate drive.

The simplest fix for me was to give "tmp_file_path[]" a fixed name in the destination directory, instead of using tmpnam() to generate a random filename in /tmp. I changed the following two functions.

BarFlyMp4TagWrite(...)
BarFlyID3WriteFile(...)

I have the same issued with /tmp on tmpfs and managed to fix it thus far with this patch I wrote. So far it works for me, hope it does for everyone else. If there's a better way for me to upload the patch, please let me know. First time doing so.

Patch

silentcontender your patch does not work for me, I get an error during make after applying your patch (to master branch):

$ make
CC src/main.c
CC src/player.c
CC src/settings.c
CC src/ui_act.c
CC src/ui.c
CC src/ui_dispatch.c
CC src/fly.c
In file included from /usr/include/fcntl.h:252:0,
from src/fly.c:32:
In function ‘open’,
inlined from ‘BarFlyMove’ at src/fly.c:1721:6:
/usr/include/x86_64-linux-gnu/bits/fcntl2.h:51:24: error: call to ‘__open_missing_mode’ declared with attribute error: open with O_CREAT in second argument needs 3 arguments
make: *** [src/fly.o] Error 1

aljex commented Dec 28, 2012

Line 1721 in fly.c , either remove O_CREAT or add the required 3rd option specifying the file creation mode (permissions, like line 619-620 (except, unlike 619, you don't want O_EXCL here.

Really just remove the O_CREAT. This line tries to open the saved mp3 or m4a file in your mp3 directory. It happens after the song is all done playing and done being ripped and saved, and now you are just updating the id3 tags. The file should always exist at this point, otherwise what are you tagging?

Old line 1721:
if ((ofp = open(file_path, O_WRONLY | O_CREAT)) == -1)

New line 1721:
if ((ofp = open(file_path, O_WRONLY)) == -1)

Also there is still a problem that the temp file is never deleted and so tmpfs /tmp fills up. Oops!!!! :)

So also add unlink(tmp_file_path); after close(ofp).
Insert a new line at 1742, after "close(ofp);" and before "return exit_status;"
Insert this new line:
unlink(tmp_file_path);

Here is the complete, fixed patch, plus a few other patches to fix the tls fingerprint and fix building dynamic.

https://github.com/aljex/misc/tree/master/pianobarfly

aljex commented Dec 28, 2012

Simpler fix, don't need BarFlyMove() or sendfile() , nor the extra unlik() to remove the leftover temp file.
Just use rename() as before, but instead of tmpnam(), just copy the regular output mp3/m4a name and append a couple characters to make the temp file name. Now temp file is always on the same fs as the destination.

https://github.com/aljex/misc/tree/master/pianobarfly
https://raw.github.com/aljex/misc/master/pianobarfly/tmp_path_fix.diff

@nega0 nega0 closed this in 03d1e61 Aug 2, 2013

markschultz added a commit to markschultz/pianobarfly that referenced this issue Oct 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment