nega0 / pianobarfly

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

All m4a files are listed as incomplete and unlinked on OSX #15

Closed filtered closed 11 years ago

filtered commented 12 years ago

I'm using Mac OSX 10.7.2 and XCode 4.2.1

After compiling everything works fine, however all files get deleted on finish because they are marked as "incomplete." if I switch src/fly.c:321 from status = unlink(fly->audio_file_path); to status = 0; //unlink(fly->audio_file_path); all the files get saved including the incomplete ones.

What can I do to fix this more correctly?

This may be the same issue as

I do get the following error message on completed files: Error opening the temporary file (Artist/Album/01 - Song.m4a/pianobarfly-tmp.m4a) (20:Not a directory). That would be a problem because that location is the source file and not a directory. This is most likely due to hunner's observation:

It appears that dirname() on Linux will modify the given path in place. On OS X (and probably other BSDs) it returns a char * to the dirname, leaving the original argument untouched.

The patch by https://github.com/mjglopez doesn't work on OSX at all. The patch by hunner at https://github.com/hunner/pianobarfly/commit/239d2aaa9c9c91359e6e199b86c0771a8f8e2c08 makes more sense, but returns

|> DEBUG: Temporary file (Artist/Album/pianobarfly-tmp.m4a) Bus error: 10

and then exits.

I have tried repairing disk permissions, but that is not the problem.

When tracing this error I get the following sequence that leads to the Bus error: 557/0x16ad: 1924178 74 71 open_nocancel("Artist/Album/pianobarfly-tmp.m4a\0", 0x601, 0x1B6) = 10 0 557/0x16ad: 1924180 2 0 lseek(0x8, 0x0, 0x0) = 0 0 557/0x16ad: 1924184 5 2 read_nocancel(0x8, "\0", 0x1000) = 4096 0 557/0x16ad: 1924187 3 1 fstat64(0xA, 0x10A41EA18, 0x10A41EADC) = 0 0 557/0x14f6: 1023 26818182 3 kevent(0x9, 0x0, 0x0) = -1 Err#4

I'm pretty sure that the 0x601 corresponds to O_WRONLY O_CREAT O_TRUNC, but I could be wrong.

neybar commented 12 years ago

I am getting the same issues. I've applied hunner's patch in both fly_id3.c and fly_mp3.c. I'm getting the bus error, then the program exits.

silverfunk commented 12 years ago

I committed a change (b8572bc4ce20720a15055dc81df759d4464c44f2) earlier tonight that may fix your problem. I replace the whole dirname() thing with tmpnam(). I've compiled it on Linux and FreeBSD but I don't have access to OSX. Give it a try and let me know if it doesn't work.

filtered commented 12 years ago

I tried the changes in b8572bc and the files still get unlinked. I also tried using the changes in 928b65c but that did the same thing.

nega0 commented 12 years ago

With trunk, I can't reproduce this on Snow Leopard.

filtered commented 12 years ago

nega0 - is this your modification of trunk? Which trunk? What version of XCode did you use to compile? There are a number of known issues with compiling this project with the version of XCode that ships with Snow Leopard. How did you fix the dirname() issue? Which issue are you unable to reproduce - the "Error opening the temporary file" or the "Bus error: 10?"

nega0 commented 12 years ago

I used a fresh checkout of https://github.com/ghuntley/pianobarfly and the only change I made (besides to the Makefile) was to add a strndup() since Snow Leopard lacks its own. Everything worked as expected. There was no dirname() issue becaue dirname() was replaced in b8572bc4ce20720a15055dc81df759d4464c44f2. I also had no Bus error or errors with the temporary file.

Also using XCode 3.2.6

You can see my minor changes here

Also I compiled with: CC=gcc CFLAGS="-std=c99 -O2 -DNDEBUG" make

filtered commented 12 years ago

Ok, so I tried that but was unsuccessful so far. That being said the protocol change has to go through for this to even work anymore. Do you (nega0) have changes that include the update to fix this issue as well as https://github.com/ghuntley/pianobarfly/pull/22 ? If I use https://github.com/nega0/pianobarfly/tree/protocol_update the stream works, but we are back to every file being deleted. I also tried the nega branch, but that does not compile for me (at least right now).

nega0 commented 12 years ago

You could try merging my protocol_update and mkstemp branches. Actually, I did it for you. You can pull my filtered branch.

filtered commented 12 years ago

I tried the filtered branch, but I am getting the Bus Error: 10 again, and then the program exits.

Occasionally it will not throw the bus error, but the file does get unlinked.

I wonder if it has something to do with Lion or XCode 4.2.1

nega0 commented 12 years ago

Unfortunatly, I can reproduce each error on Lion (bus error, and incomplete/unlink) but not consistently. I was able to trigger the bus error under gdb. It occurred during the recursive call to _BarFlyMp4AtomRender() (fly_mp4.c:773). A call to _BarFlyMp4LongRender() (fly_mp4.c:705) couldn't happen because it couldn't access "buffer". "Can't access memory at address 0xXXXXXXXXX".

Got it to crash again with a printf(atom->name) at the beginning of _BarFlyMp4AtomRender():

(i) Receiving new playlist... Ok.
|>  "Everybody Knows" by "Leonard Cohen" on "The Essential: Leonard Cohen" <3
** moov **/05:36   * Recording
** mvhd **
** iods **
** trak **
** tkhd **
** mdia **
** mdhd **
** hdlr **
** minf **
zsh: bus error  MallocStackLogging=1 ./pianobarfly
nega0 commented 12 years ago

And another, this time in gdb:

|>  "Better Off Without A Wife (Live)" by "Tom Waits" on "Nighthawks At The Diner"
(i) Loving song... Ok.
** moov **/03:59   * Recording
** mvhd **
** iods **
** trak **
** tkhd **
** mdia **
** mdhd **
** hdlr **
** minf **

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x000000010046d2a8
[Switching to process 19612 thread 0x8e13]
0x00000001000111a6 in _BarFlyMp4AtomRender (atom=0x101b13520, in_file=0x7fff78aa7f78, out_file=14, settings=0x100037c88) at fly_mp4.c:702
702         printf("** %s **\n", atom->name);
(gdb) 

And the backtrace:

#0  0x00000001000111a6 in _BarFlyMp4AtomRender (atom=0x101b13520, in_file=0x7fff78aa7f78, out_file=14, settings=0x100037c88) at fly_mp4.c:702
#1  0x00000001000115f8 in _BarFlyMp4AtomRender (atom=0x101b134e0, in_file=0x7fff78aa7f78, out_file=14, settings=0x100037c88) at fly_mp4.c:775
#2  0x00000001000115f8 in _BarFlyMp4AtomRender (atom=0x101b13420, in_file=0x7fff78aa7f78, out_file=14, settings=0x100037c88) at fly_mp4.c:775
#3  0x00000001000115f8 in _BarFlyMp4AtomRender (atom=0x101b121e0, in_file=0x7fff78aa7f78, out_file=14, settings=0x100037c88) at fly_mp4.c:775
#4  0x00000001000115f8 in _BarFlyMp4AtomRender (atom=0x101b11e70, in_file=0x7fff78aa7f78, out_file=14, settings=0x100037c88) at fly_mp4.c:775
#5  0x0000000100014b45 in BarFlyMp4TagWrite (tag=0x101b139e0, settings=0x100037c88) at fly_mp4.c:1969
#6  0x000000010000efd1 in _BarFlyTagMp4Write (fly=0x100037950, cover_art=0x102814800 "����", cover_size=46048, settings=0x100037c88) at fly.c:1252
#7  0x000000010000f1a6 in _BarFlyTagWrite (fly=0x100037950, settings=0x100037c88) at fly.c:1299
#8  0x000000010001025c in BarFlyTag (fly=0x100037950, settings=0x100037c88) at fly.c:1720
#9  0x0000000100003b02 in BarPlayerThread (data=0x100032790) at player.c:478
#10 0x00007fff8fd168bf in _pthread_start ()
#11 0x00007fff8fd19b75 in thread_start ()
(gdb) 

"atom" works out to:

12: atom->offset = 370
11: atom->data_size = 8
10: atom->data = (uint8_t *) 0x0
9: atom->child_count = 0
8: atom->children = (BarFlyMp4Atom_t **) 0x0
7: atom->parent = (BarFlyMp4Atom_t *) 0x101b134e0
6: atom->size = 16
1: atom->name = "smhd"

It seems to have recovered and successfully tagged though. Hmm...

silverfunk commented 12 years ago

I recreated our old develop branch and committed up a change that will hopefully fix this once and for all (267c0b14c2cdaf1a90346be4f86a2f793a10d09b). I honestly don't know what I was thinking about the hole dirname() and tmpnam() stuff. If you look at the way the code works the tmp file can just be created in the root of the current working directory, which is always the base directory where files are being written. So basically this change just creates the tmp file in that directory and then move it to the final destination.

Give the fix a try and let me know what you think.

madallday commented 12 years ago

OS X 10.7.3 tried master and develop branches

compiled with make clean && make CFLAGS="-O2 -DNDEBUG -W6 4" DISABLE_ID3TAG=1

both giving me Bus error: 10:43 and exits consistently after first song.

filtered commented 11 years ago

This is still an issue with the current commit and on OS X 10.8.2.

eklein commented 11 years ago

still an issue with 10.7.5 as well.

nega0 commented 11 years ago

Is this still an issue?

eklein commented 11 years ago

I still get a bus error.

osx 10.8.4 now.

nega0 commented 11 years ago

On OSX 10.9 DP4 w/ the current XCode preview, I'm not getting the bus error, but I am seeing the unlink issue. I'm only about 10 songs in though

eklein commented 11 years ago

What build options did you use? and how did you install the rest of the dependencies? I used brew install to install pianobar first to get most of the dependencies..

nega0 commented 11 years ago

I put a basic page up on the wiki

eklein commented 11 years ago

re-installed all the packages following your basic guidelines, then rebuilt pianobarfly using your command-line and i'm still getting the bus error. :(

I'm happy to help debug if there's any additional info that might help.

nega0 commented 11 years ago

The following patch will work around the unlink issue on OSX

diff --git a/src/libwaitress/waitress.c b/src/libwaitress/waitress.c
index c37cc2c..75ff84a 100644
--- a/src/libwaitress/waitress.c
+++ b/src/libwaitress/waitress.c
@@ -988,6 +988,7 @@ static WaitressReturn_t WaitressReceiveHeaders (WaitressHandle_t *waith,
                    switch (WaitressParseStatusline (thisLine)) {
                        case 200:
                        case 206:
+                       case 400:
                            hdrParseMode = HDRM_LINES;
                            break;
nega0 commented 11 years ago

Anyone who's seeing this bus error, I need some info. Make sure your sources are up-to-date, run pianobarfly, and let it crash. You should have (part) of the tmpfile in your "mp3 directory" (by default, ./mp3). It should be 394 bytes. The easiest way to do this is to run xxd pianobarfly-08VLkR | pbcopy and then Cmd-V in the comment text area. (Your tmpfile name will obviously be different.) You'll then get a block like this:

0000000: 0000 0024 6674 7970 6d70 3432 0000 0000  ...$ftypmp42....
0000010: 6d70 3432 6973 6f6d 3367 7036 3367 3261  mp42isom3gp63g2a
0000020: 3367 7034 0001 409f 6d6f 6f76 0000 006c  3gp4..@.moov...l
0000030: 6d76 6864 0000 0000 c3ff 1a88 c3ff 1a88  mvhd............
0000040: 0000 0258 0002 ae95 0001 0000 0100 0000  ...X............
0000050: 0000 0000 0000 0000 0001 0000 0000 0000  ................
0000060: 0000 0000 0000 0000 0001 0000 0000 0000  ................
0000070: 0000 0000 0000 0000 4000 0000 0000 0000  ........@.......
0000080: 0000 0000 0000 0000 0000 0000 0000 0000  ................
0000090: 0000 0000 0000 0002 0000 0021 696f 6473  ...........!iods
00000a0: 0000 0000 1080 8080 1000 4fff ff0f ffff  ..........O.....
00000b0: 0e80 8080 0400 0000 0100 006e 2f74 7261  ...........n/tra
00000c0: 6b00 0000 5c74 6b68 6400 0000 07c3 ff1a  k...\tkhd.......
00000d0: 88c3 ff1a 8800 0000 0100 0000 0000 02ae  ................
00000e0: 9500 0000 0000 0000 0000 0000 0001 0000  ................
00000f0: 0000 0100 0000 0000 0000 0000 0000 0000  ................
0000100: 0000 0100 0000 0000 0000 0000 0000 0000  ................
0000110: 0040 0000 0000 0000 0000 0000 0000 006d  .@.............m
0000120: cb6d 6469 6100 0000 206d 6468 6400 0000  .mdia... mdhd...
0000130: 00c3 ff1a 88c3 ff1a 8800 00ac 4400 c520  ............D.. 
0000140: 0055 c400 0000 0000 2568 646c 7200 0000  .U......%hdlr...
0000150: 0000 0000 0073 6f75 6e00 0000 0000 0000  .....soun.......
0000160: 0000 0000 0073 6f75 6e00 0000 6d7e 6d69  .....soun...m~mi
0000170: 6e66 0000 0010 736d 6864 0000 0000 0000  nf....smhd......
0000180: 0000 0000 0024 6469 6e66                 .....$dinf

This will let me know if it's crapping out in the same place for everyone.

nega0 commented 11 years ago

On the unlink issue, WaitressPollRead() is somehow fucked on OSX. This line is erroring with a Connection reset by peer. I'm guessing we're trying to read more than the content-length, and we're getting to told to GTFO. Anyway, this then sets WAITRESS_RET_READ_ERR which trickles back up to here, causing us to to request data from after the last byte of the content-length, the server sends back an HTTP 400 (Bad Request), libwaitress/waitress.c freaks out because it doesn't know what to do with a 400, and we delete the file.

This is probably easily fixed by making sure we don't try to request a range greater than the content-length, but I want to know what's triggering the Connection reset by peer, and why only on OSX.

This all masks the SIGBUS error (because the file never gets tagged)...

eklein commented 11 years ago

@nega0, swapped out laptops, need to get my environment all setup again and then I can get you some data. do you want me to apply the patch before collecting data?

eklein commented 11 years ago

btw, on your wiki, you mention installing "faad", which I don't see as an available brew package.

eklein commented 11 years ago

I didn't apply the patch, but I built the most current sources pulled down from github.

0000000: 0000 0024 6674 7970 6d70 3432 0000 0000  ...$ftypmp42....
0000010: 6d70 3432 6973 6f6d 3367 7036 3367 3261  mp42isom3gp63g2a
0000020: 3367 7034 0000 14ae 6d6f 6f76 0000 006c  3gp4....moov...l
0000030: 6d76 6864 0000 0000 c3f4 f376 c3f4 f376  mvhd.......v...v
0000040: 0000 0258 0000 6d0f 0001 0000 0100 0000  ...X..m.........
0000050: 0000 0000 0000 0000 0001 0000 0000 0000  ................
0000060: 0000 0000 0000 0000 0001 0000 0000 0000  ................
0000070: 0000 0000 0000 0000 4000 0000 0000 0000  ........@.......
0000080: 0000 0000 0000 0000 0000 0000 0000 0000  ................
0000090: 0000 0000 0000 0002 0000 0021 696f 6473  ...........!iods
00000a0: 0000 0000 1080 8080 1000 4fff ff0f ffff  ..........O.....
00000b0: 0e80 8080 0400 0000 0100 0012 ff74 7261  .............tra
00000c0: 6b00 0000 5c74 6b68 6400 0000 07c3 f4f3  k...\tkhd.......
00000d0: 76c3 f4f3 7600 0000 0100 0000 0000 006d  v...v..........m
00000e0: 0f00 0000 0000 0000 0000 0000 0001 0000  ................
00000f0: 0000 0100 0000 0000 0000 0000 0000 0000  ................
0000100: 0000 0100 0000 0000 0000 0000 0000 0000  ................
0000110: 0040 0000 0000 0000 0000 0000 0000 0012  .@..............
0000120: 9b6d 6469 6100 0000 206d 6468 6400 0000  .mdia... mdhd...
0000130: 00c3 f4f3 76c3 f4f3 7600 00ac 4400 1f50  ....v...v...D..P
0000140: 0055 c400 0000 0000 2568 646c 7200 0000  .U......%hdlr...
0000150: 0000 0000 0073 6f75 6e00 0000 0000 0000  .....soun.......
0000160: 0000 0000 0073 6f75 6e00 0000 124e 6d69  .....soun....Nmi
0000170: 6e66 0000 0010 736d 6864 0000 0000 0000  nf....smhd......
0000180: 0000 0000 0024 6469 6e66                 .....$dinf
nega0 commented 11 years ago

booya! got it. need to do up a clean patch.

nega0 commented 11 years ago

re: "faad"

I probably typo'd "faac". My current brew list follows. Obviously, not all are needed, and some are dependencies of what I've listed.

[badtz:~/pianobarfly] brew list
ack
ccache
cmake
faac
faad2
ffmpeg
gmp
gnutls
json-c
lame
libao
libgcrypt
libgpg-error
libid3tag
libtasn1
mad
nettle
p11-kit
pkg-config
texi2html
wget
x264
xvid
xz
yasm
eklein commented 11 years ago

@nega0, hot! I'll give it a shot shortly.

eklein commented 11 years ago

No such luck.. it's no longer getting a bus error, but it's still deleting "partially recorded file"

nega0 commented 11 years ago

add the patch in https://github.com/nega0/pianobarfly/issues/15#issuecomment-23385643

that'll work around it until i figure out what the real issue is and fix that

eklein commented 11 years ago

OK, trying now :) thanks for all your work on this!

eklein commented 11 years ago

So there's no bus error and it's not saying "deleting partially recorded file", but files and the associated subdirectories aren't being written to the mp3 directory.

nega0 commented 11 years ago

where's your mp3 directory and how are you running pianobarfly?

For instance, im running in it in the source directory using the default mp3 directory of "./mp3". So, I end up with

[badtz:~/pianobarfly] ./pianobarfly
[badtz:~/pianobarfly] ls
COPYING      ChangeLog    INSTALL      LICENSE      Makefile     README       contrib/     mp3/         pianobarfly* src/
[badtz:~/pianobarfly] find mp3
mp3
mp3/Autolux
mp3/Autolux/Future_Perfect
mp3/Autolux/Future_Perfect/Autolux-Plantlife.m4a
eklein commented 11 years ago

ugh, sorry.. i realized I reverted back to a stock config file just to make sure my old config file wasn't causing problems.. it was placing them in ./mp3 just like you said. The files are there :)

Great work! Please let me know if I can help with debugging anything else or testing a fix.