Closed (fixed)
Project:
Audio
Version:
5.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
8 Sep 2005 at 20:09 UTC
Updated:
6 Oct 2005 at 23:28 UTC
Jump to comment: Most recent file
songs stop playing before finish
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | geary-2005-09-11.audio.module | 31.55 KB | Geary |
| #8 | geary-2005-09-11.audio.patch | 2.99 KB | Geary |
| #5 | geary-2005-09-09.audio.module | 32.05 KB | Geary |
| #4 | geary-2005-09-09.audio.module.patch | 809 bytes | Geary |
Comments
Comment #1
Geary commentedSame thing happens here, quite consistently. Songs always stop at exactly the same time. Some hard coded limit somewhere?
Comment #2
Colin Brumelle commentedThanks 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
Comment #3
Geary commentedColin, 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...
Comment #4
Geary commentedredpineseed, 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.
Comment #5
Geary commentedAnd here's an already-patched audio.module in case anyone wants it for convenience.
Comment #6
Geary commentedUpdated status
Comment #7
redpineseed commentedi'd set the second arg of file_transfer to null, to stop the watchdog complaining.
Comment #8
Geary commentedSilly 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.
Comment #9
Geary commentedAnd 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.)
Comment #10
redpineseed commentedthat is great Geary. good work.
Comment #11
Colin Brumelle commentedI've applied the latest patch and commited an updated audio.module to CVS. Thanks for finding this bug, and for fixing it!
Comment #12
(not verified) commentedComment #13
Colin Brumelle commented