Comments

Geary’s picture

Same thing happens here, quite consistently. Songs always stop at exactly the same time. Some hard coded limit somewhere?

Colin Brumelle’s picture

Assigned: Unassigned » Colin Brumelle

Thanks for the bug report! No hard coded limit in the audio module itself... Perhaps some more details about the problem would help me track it down though. To clarify here, you're talking about the song not playing in the flash player all the way through? What happens when you download the song? Does it stop in the same place? As a first guess, this might be caused by the 'upload limit' in your php.ini file (or the memory limit). If the song stops in the same place when you download, it would support this hypothesis. It might also be caused by an upload limit in the apache configuration file.

If possible, could you email me the file (colin@bryght.com) so I can try and reproduce the bug...

Thanks for bringing this up!

C

Geary’s picture

Colin, I found the problem and fixed it--very simple fix. Will submit a patch in just a bit; just wanted to give you a heads-up...

Geary’s picture

StatusFileSize
new809 bytes

redpineseed, I'll bet that the following are true:

1) You are running PHP 5.0.4, and

2) If you download one of your audio files through the "download audio file" link, you will see that it is exactly 2,000,000 bytes long.

There is a bug in PHP 5.0.4 which causes the readfile() function to truncate the file at exactly 2,000,000 bytes. Here are some discussions of this problem:

http://www.jamroom.net/phpBB2/viewtopic.php?p=24066
http://www.mail-archive.com/internals@lists.php.net/msg14185.html
http://us2.php.net/manual/en/function.readfile.php
http://forum.textdrive.com/viewtopic.php?pid=45606

To fix this, I changed the readfile() calls in audio.module to use Drupal's file_transfer() function instead. This function reads the file in chunks using fread() so it does not encounter the problem. A patch is attached.

The file_transfer() function takes a second parameter which is an array of headers to be sent. I didn't change the code to use this, just to keep the patch simple. But it might make sense to to that instad of calling header() directly in the audio module.

Geary’s picture

StatusFileSize
new32.05 KB

And here's an already-patched audio.module in case anyone wants it for convenience.

Geary’s picture

Status: Active » Needs review

Updated status

redpineseed’s picture

i'd set the second arg of file_transfer to null, to stop the watchdog complaining.

Geary’s picture

StatusFileSize
new2.99 KB

Silly me, what was I thinking? Note to self: Check the admin log for PHP warnings!

Here's an updated patch. This time I refactored the nearly-identical audio_fetch() and audio_play() functions so that they use a common function. I also changed the code to pass a $headers array into the file_transfer() call intead of calling header() within these functions. To allow this, I combined the two Cache-Control header calls into one so that it would not need the second false argument in the header() call. This makes no functional difference because PHP collapses those two headers into one anyway.

Geary’s picture

StatusFileSize
new31.55 KB

And here again is a pre-patched copy of audio.module for convenience. (I hope I don't annoy anyone by posting these as well as the patches--I've seen a few requests from people asking for already-patched files in other modules.)

redpineseed’s picture

that is great Geary. good work.

Colin Brumelle’s picture

Status: Needs review » Fixed

I've applied the latest patch and commited an updated audio.module to CVS. Thanks for finding this bug, and for fixing it!

Anonymous’s picture

Colin Brumelle’s picture

Version: » 5.x-1.x-dev
Status: Fixed » Closed (fixed)