Update to 4.7

Here's a patch, based on my forms api update (http://drupal.org/node/38880), that uses filemanager as a backend. I started working on it but got busy on other stuff so I don't really rememeber what works and what doesn't. I'll put it up here as a starting point.

Well I've got a big chunk of it working. The previewing works and the ID3 tags are read and written correctly.

It's getting close to a full rewrite so I won't bother with a patch (the patch is bigger than the module).

The database table is different so I changed the name, it'll probably get changed again so don't put in anything you're concerned about ;)

here's the db

and here's a file with the themes in it. i got sick of them being in the way.

wow.. this is a great idea.. look forward to trying it out! thanks for all your work on this drewish....

hmmm... i don't know... for some reason when you said filemanager.module i was thinking upload.module... i've yet to use the filemanager module, so I'm trying it out now on 4.7.beta2....

a few concerns:

what happened to the flash play button?
when i click on play i receive this error:

    * warning: filesize(): Stat failed for files/active// (errno=2 - No such file or directory) in htdocs/modules/filemanager/filemanager.module on line 529.
    * warning: fopen(files/active//): failed to open stream: No such file or directory in htdocs/includes/file.inc on line 469.

q0rban, i haven't looked at any of the flash stuff yet.

i think the streaming stuff may be borkd too. i've got some patches for the filemanager module that need to get commited. i may just fork a copy and put it up in a sandbox.

I've put the module in progress up in my sandbox: http://cvs.drupal.org/viewcvs/drupal/contributions/sandbox/drewish/audio/

Feedback is very welcome.

I've made quite a few improvements to the branch of the audio module in my sandbox.

One thing I'm pretty happy with is the ability to browse for audio by ID3 tags. You try it out on a website I'm working on: http://131.252.78.131:8080/audio/by/

Browsing by ID3 tags is sweet! Great looking site BTW...

While filemanager may offer interesting functionality, I have two concerns:
1. Audio is not complex enough to have to depend on anything other than core functionality
2. Why don't you just submit patches against the core File API rather than depending on a contrib module?

The File API is definitely in need of some work. Let's make core that mutch better and move things forward for everyone.

And drewish...thanks for all the awesome work you've put into audio.module.

I think it's right to question the use of the filemanager module. The two things that made the decision for me were:

• it was easy to add and keep track of the temp file during previews and then "commit" them when the node was saved.
• it was easy to "embed" the upload functionality, rather than having to enable file uploads on a per-node type basis.

The upload module may be able to do both of these when I made the choice, I either couldn't figure it out, or it wasn't do-able.

I'd really like to see some changes made to the upload module but I'm probably not the one to do it ;) That, and I wanted to get the module working for a site I'm buillding. I didn't/don't have time to do the politicking needed to get the major changes made in 4.7 that upload would need. I know that Ber has talked about a "betterupload" module that builds on the improvements in filemanager. It seems like it's still in the planning stages but it looks very promising.

I'm thinking about changing the way the node's title is handled. I'd like to have it generated from the file's metatdata. Perhaps using a setting that's passed to t() so the admin can choose the what will be used to construct the title.

Does anyone have any thoughs on this? How do people use the title field?

I know we've discussed this before, but I think it makes sense to have a seperate field for the title... The audio file is still basically an attachment to a node, so It makes the audio node type much more versatile... Having it automatically set the title from the metadata, although it may save a step, takes away some of the flexibility of the audio.module IMO...

The real question is, are we willing to answer all the support topics entitled "How do I change the title of an audio node?"

Or, maybe there's another way... Can we create a way to attach audio to other content types, for instance a 'page'? or even giving users the ability to attach audio files to forum posts.. wow, that would be awesome.. that way you can create any node that has the audio file, metadata, and flash player attached to it. If a user were to create an audio node, it will fill all the fields from the metadata (including the title), but other node types (obviously) have user submitted title fields... I don't know if there are hooks for this or not...

cheers

Hi drewish,

I'm glad I read your issue this morning. I was actually planning to do the same thing: use filemanager with the audio module. So, I'm on board to test the audio fork in your sandbox!

I'm testing on a LAMP (php 4.11) and Drupal 4.7 beta 3.
Yet, I had already one minor (php version related) bug which I made a patch for (see attached).

Drewish,

Is it possible to commit your audio table structure in your sandbox or post it? It seems you latest sandbox audio module is not compliant with the audio.mysql file you posted earlier in this issue.

Thx

q0rban, you talked me into what's probably the best of both worlds. I just checked in a whole mess of code. The big new feature is that the titles can now use the meta data as variables like "%title by %artist". You can set the default title in admin/settings/audio. But, if you just want to type in a title you can do that.

On your second point of letting other nodes encapsulate the audio node, I'd like to keep that in mind but I think it'll probably happen when a more generic upload/attachment module is built. Perhaps it'll be Ber's betterupload module.

toemaz, sorry, that .mysql file is pretty old. the easiest thing to do is drop those tables and then use update.php to install and upgrade the tables (i think i added docs for it in the INSTALL.txt file). Thanks, for the patch but I ended up deleting the code you'd fixed. I found the redirect when there was just a single node confusing.

Ok, thx for the tip. It worked well.

bugreport for drewish sandbox module:

In your menu hook, this item doesn't work

$items[] = array(
'path' => 'audio/user', 'title' => t("user's audio files"),
'access' => user_access('access content'),
'type' => MENU_NORMAL_ITEM,
'callback' => 'audio_page_user');

Or you want to change it into

'path' => 'audio/user/'.$user->uid, 'title' => t("my audio files"),

Or I don't know what the reason is for the menu item

Also, when editing existing audio files, the title gets replaced with the following:

audio_default_title_format

  if (!isset($node->title_format)) {
    $node->title_format = variable_get('audio_default_title_format', '%title by %artist');
  }
  $form['title_format'] = array(
    '#type' => 'textfield', '#title' => t('Title'),
    '#default_value' => ($node->title_format) ? $node->title_format : 'audio_default_title_format',
    '#description' => t("The title can use the file's metadata. Depending on what's filled out, you maybe able to use any of the following variables: " ) . '%artist, %album, %title, %track, %genre, %year',
    '#required' => TRUE,
    '#weight' => -10,
  );

under '#default_value', should the FALSE state be a call to the variable_get() function? Not sure why $node->title_format is evaluating as FALSE anyways, unless it's set as nothing ('')

cheers

I also receive the following error when submitting new audio or editing existing audio:

warning: implode(): Bad arguments. in /modules/audio/getid3/write.php on line 465.

cfr. previous bug: I reproduced the same fault.

toemaz, that audio/user path isn't working right now. I want to try to move the user pages and feeds into a separate menu item. If you've got any ideas I'd like to hear them. I've committed the other patches you sent.

q0rban, thanks for pointing that out, I'd only half finished the change. It's fixed. Also, if you figure out anything about setting the flash button's background color let me know.

Regarding the "Bad arguments. in /modules/audio/getid3/write.php on line 465." warnings. That's a bug in the getID3 package. Rather than checking if there's values for the ID3v1 tags, just just try to suppress the warnings. I'll open a bug on their site when I get a chance.

Thanks for the feedback, please keep it coming.

One additional datapoint on the whole filemanager dependency issue. Based on this posting, it looks like filemanger is one of the most popular Drupal 4.6 modules. That'd make it much less of a burden for lots of sites.

Just a follow up on #21 and #22, I've posted a bug report and patch the getID3. I expect that it'll be included in the 1.7.6 release.

Sorry, one more post. Wanted to mention a couple of other things:

• q0rban figured out how to get rid of the white box around the flash player and you've got no idea how stoked I am about it. The button itself accepts parameters for the colors during various states (somthigg like playing, stopped, downloading, etc), I want to make it easier for themes to specify these colors.
• I'm starting on some code to allow the admin to select a list of allowed tags. That way you can can add additional ID3v2 fields (like recorded date), or remove tags you don't want (say, genre). A side effect will be that you can select the order of the meta data fields.

Very nice. Be sure you don't push it to far ;-) Better to have a nice stable release...
I'm following your updates!

This is all awesome...

When right clicking on the 'download audio file' link and save the link location, the saved filename is 'download audio file.htm'.
Changing the extension to mp3 (if it is an mp3 file), solves the problem if you want to open it locally.

But, is there no solution to push another filename? Through the header somehow? (I remember this should be possible, but it has been some time ago). Since you are using the filemanager, this issue is perhaps a filemanager issue.

"The button itself accepts parameters for the colors during various states (somthigg like playing, stopped, downloading, etc), I want to make it easier for themes to specify these colors."

Can we tie this into the style sheet perhaps? I think that is the best place for color parameters... Maybe just pick off the same color as whatever parent element the flash player is sitting in? I can't imagine why anyone would ever want to see a different colored ugly box around their player, and I can't wait to get rid of the box!

[I had a nice response all written up but Drupal though it was an XSS attack and discarded my preview. That'll teach me not to copy and paste.]

All it takes to get rid of the white box is adding < param name="wmode" value="transparent" / > to the player's <object> element. We can commit this to 4.6 if you'd like.

In regards to the button coloring. The colors are specified either in the parameters.as file or in the URL passed to the player. The parameters are:

• 'b_bgcolor' the (in circle) backgroud color
• 'b_fgcolor' the (circle) border and icon color
• 'b_colors' four colors for the different states: connecting (spinner), ready (play arrow), playing (stop square), ??? (not sure). If any anyone is omitted, the b_fgcolor is used in it's place.

I don't think we'll be able to pull these values from a style sheet. My thought was that a theme could override theme_audio_mp3_player() and then call it and pass in the colors that they it would like to use.

I was trying to use this module but it wouldn't recognize the uploaded file.

It just gave the error: A file must be provided.
Acidfree works to upload pictures but this is not working at all. Anyone have any suggestions ?

My only guess would be to try using the filemanager.module in drewish's sandbox... I think he has some patches on the clean version..

metapunk, most everything (besides the streaming) should work with the HEAD version of filemanager. Older versions may not work.

Make sure you've got the current version of Drupal, filemanager from HEAD or my sandbox and the audio module from my sandbox.

If you're still having problems with it, please contact me via my user profile.

Okay, it looks like metapunk's problems may be related to PHP's upload_max_filesize and post_max_size variables. Both of these limit the sizes of file uploads. I'm going to try to add some code the audio form to print these limits.

Oh, one other thing. I added calls to db_rewrite_sql() in the browsing code so now permissions will be honored, i.e. you won't see meta data for nodes you can't view.

The description of the upload button now displays the maximum allowed upload.

The description of the upload button now displays the maximum allowed upload.

Maybe filemanager should display that under any upload button, instead of just the audio module... Or is that not possible?

cheers...

filemanager doesn't really provide the upload code, it just handles keeping track of files and the space they take up. i the code to compute the maximum upload really seems like something that should be in core some place.

I'm thinking that the node teaser needs to be reworked. The default of (roughly) %title by %artist isn't as useful now that the title can be built from variables. Any one have any suggestions on what it should be?

A couple of updates:

• Added a 'Browse by' block that lists the tags to browse by.
• Recent and Random blocks are now configurable, you can select the number of nodes to display.
• Updated the help text on admin/help/audio.
• Added a warning note to the "enable downloads" checkbox, since it really just makes it mildly harder for someone to download the file (the flash player actually does it for you).
• The flash player now uses an absolute URL (because relative paths are no more) and is only used for 44, 22, and 11 kHz MP3s.

1. I little problem came up with your latest sandbox version. Whenever I want to add/edit a audio node, I get the following error:

warning: htmlspecialchars() expects parameter 1 to be string, array given in C:\xx\xx\includes\bootstrap.inc on line 714.

Can anyone reproduce it?

2. Another little one:

When editing an audio node by changing the audio file, the 'current file' name in the 'audio file' fieldset: $form['audio_fileinfo']['filename']
is not being changed into the new filename.

3. 'Allow file downloads' checkbox does not preserve its value when previewing the form (after clicking the preview button).

Afterall, minor bugs, but might be annoying. Anyhow, great work Drewish!

toemaz, Thanks for the feedback.

I can confirm #1. Not sure what the cause is. I'd just attibuted it to the lastest updates to Drupal HEAD. I'll look into it.

On #2, I don't think the filename will stick until you save. Not sure if there's an easy fix for that but it would be a good thing to fix.

#3, Hadn't seen that. I think there may still be a bug with checkboxes in HEAD. I'll let it settle down a bit before tackling that one.

On #2, I don't think the filename will stick until you save. Not sure if there's an easy fix for that but it would be a good thing to fix.

It actually does (I just checked it again). I couldn't check out the code yet... might be something with the filemanager.

Possible solution for #3:
'#default_value' => ($node->audio_fileinfo['downloadable'] == 1 ? true : false),

But I guess there is a much better way.

I've changed the way the default_value for the download link is set, it's pretty much what you suggested toemaz.

I'm not clear now, are you still having a problem with the filename not updating?

That htmlspecialchars() warning looks like it's because we don't use a title in the proper manner. I'm working on a fix for that.

I was wrong about the htmlspecialchars warning, it was due to the form's id tag. It was getting merged in with an existing one and being put into an array which check_plain wasn't expecting. I removed the id and the warning is gone (and the form still has an id).

I did a little testing on that filename issue. I think that's a problem with filemanager. I'll open an issue over there.

Keep 'em coming,
andrew

Been working on a few different things:

• Got the install hook working in the audio.install file. Anyone use Postgres? Want to do schema for me?
• Added a setting to choose which metadata fields you'd like and their ordering. There's still quite a bit to do on this. I need to put some "hooks" in to allow appropriate themeing of the different tag types.

• Made the browseable tags setting apply to both the block and regular browsing.
• Undefined values in the title now show up as '' rather than %value
• Added a teaser format setting that works similarly to the title so you can modify the teaser generation without having to override the theme function.

So far as I tested, it works fine!
I'm using your latest sandbox version all the time. So keep on going ;-)

great, i'm glad to hear it. i'm currently trying to track down a bug that prevents you from removing comment tags. please let me know if you run into anything wierd with additional id3 tags.

Alright. Didn't test the tag's so far. Will keep my eye on it.

BTW seems like the filemanager maintainer replied on a lot of open issues. Good evolution for this audio module according to me, knowing filemanager has a future.

Next up:
* hook_search, using zirafa's patch as a starting place
* probably implement hooks for the views module, i think it''ll open up a lot of functionality.

hello,
for some reason the .install thing doesnt work on my 4.7b4 system (yet). could you maybe provide the .mysql file so i could still install the module? many thanks!

jeez i changed the title.. hope this changes it back. sorry

yeah, i guess the hook_install didn't make it into beta4. if you open up the .install file in audio_install() you'll find a copy of the database schema.

Bryght has hired me to remove the filemanger dependency. Most of the work is complete (stay away from revisions though) and checked into my sandbox. I want to do a bit more testing before committing it back to the "official" module.

To use this, due to a recent change in the core file tables, you must be running a recent version of HEAD (update 173 or greater). I've added an update to audio.install that migrates existing audio nodes from filemanager to core. You won't even see the update if you're not running a new enough version of Drupal.

Warning: I've tested it but I won't guarantee that the upgrade will work for you. If this scares you, make a backup of your database and files directory first.

I've found a few different bugs in the getID3 library. The maintainer has fixed most of them but hasn't released a newer version. He sent me a current copy of the development version which I'm attaching. If you run into fatal errors with the 1.7.5 version, try using this instead.

receive this error when running the update script:

Fatal error: Only variables can be passed by reference in /home/user/public_html/modules/audio/audio.install on line 141

Using update 174 of drupal and your most recent sandbox version..

i forgot to mention that i've fixed the update. q0rban reported back via email that it worked for him.

also, i'm starting work on supporting the views module.

* Something with the uploads was "fixed" in core. I've made some changes to get it working. The uploads are now moved to drupal's temp directory and then when the node is saved to files/audio.
* Improved reporting of ID3 read write errors and warnings. This may be annoying but it'll help trackdown errors in the getID3 library.

I receive the following error when trying to edit an audio node:

A file must be provided.

q0rban, thanks. that's what i get for committing without doing that final testing. it should be fixed, i've also improved the warning messages.

following moshe's suggestion, i've moved the audio files to their own table. now there's no more conflicts with the upload module over who owns what file. revision should work now but you'll get copies of each file. the revisions aren't playable or downloadable as the urls use the node id not the version id. i'll probably need to break backwards compatiblity to get that working.

we're getting pretty close to commitable.

well, i've gone and done it. the re-write has been committed to HEAD.

anyone who's been using my sandbox version should update to the latest version, then run Drupal's update.php script to make sure their tables are up to speed. once that's finished, remove the sandbox audio module and checkout a clean copy from HEAD.

please open new issues for any bugs you find from this point on.

quick question: did you update the 4.6 branch before committing the 4.7 changes, or is it possible to do that?

there's a branch for 4.6, this is commited to head. there's a bug in the project module that's screwing with the listed release (http://drupal.org/node/47158).

Nicely done!
Thanks for commiting to head.

C