nega0 / pianobarfly

pandora2[mp3|m4a]
https://github.com/nega0/pianobarfly
Other
62 stars 31 forks source link

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

Closed spockout closed 11 years ago

spockout commented 12 years ago

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(...)

silentcontender commented 12 years ago

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

copocaneta commented 12 years ago

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

ghost commented 11 years ago

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

ghost commented 11 years ago

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