Greetings,

We are developing audiofield and videofield for CCK now. They are almost completed and soon be available for community. We found that imagefield and filefield have lot of common things. Our audio and video fields also have common parts which are ( of course :) ) similar to imagefield. We are working on extracting them into common INC file which may be used in all 4 fields and may be in other future fields which will deal with file upload.

I wonder, will you be interesting in refactoring image and file CCK fields using this common INC file and what ideas and thoughts do you have concerning this question? May be you already did this ... in this case we will be interested in using it for our media fields.

Thank you for your feedback.

Comments

yched’s picture

Duke, this sounds really promising.
Another user, agilpwc, has submitted a video field for cck over there : http://drupal.org/node/105119.

It would be really nice if both of you could get in touch and merge your efforts so that we don't end up with two different modules for the same functionality :-)

quicksketch’s picture

+1 on this.

Rather than a .inc file used by all three, maybe filefield should be the primary container of this code. It seems like that would be a reasonable dependency. Imagefield has seen far more updates than filefield however, so I believe that it's going to have the most complete feature set to base this new code on.

dopry’s picture

I'd love to see what you have! I originally intended for stuff like that in filefield. I'd be happy to include it in the filefield.module.

.darrel.

ardas’s picture

Okay. I'll look at that new video module and shortly upload both of my video and audio modules for you to review.

ardas’s picture

Well, I studied the module created by agilpwc today and found that he was focusing on how to display video data. When I was developing video module I was focusing on how to read and store video data. I still have no player at all and I didn't solve this task yet. My idea is not to hardcode any player but create something to customize the way video data is represented. On the other hand I have more columns for my field where all video specific data are stored. So, you can search video by time, encoding, etc. I'm using getid3 lib to read them.

I did both video and audio fields.

These modules were made from filefield and imagefield.
I created a project here http://drupal.org/project/mediafield.

How about we continue our mutual work upon this project and probably their common part together? We created an INC file which you may use in your filefield.module and all other file fields can depend on your one.

I did these modules for Drupal 4.7 for now. Release will be ready this night.

dopry’s picture

Awesome work... I was looking a the .inc... It would be cool to include this in filefield. I can also write a function to fix a file to the session so preview urls will work, since I'll have a hook_menu in filefield. I'll try to get on it next week. I have pending work on cck and nodereferrer that need to be done this week.

Egon Bianchet’s picture

Version: 4.7.x-1.x-dev » 6.x-3.x-dev

I submitted a patch to filefield http://drupal.org/node/123532 to have an initial split between filefield.module and filefield.inc

jpetso’s picture

Status: Active » Reviewed & tested by the community
StatusFileSize
new11.38 KB

The issue mentioned by Egon has been closed, with dopry stating that he will not split out logic into an .inc file, instead modules should be depending on filefield itself, and filefield.module will contain the required functions. Currently, the modifications needed for sharing code are a part of filefield's revision support patch, as both goals require changes to the same parts of the code.

The attached patch makes imagefield use filefield's file_insert(), file_update(), file_load(), and file_delete() functions, and depends on filefield's revision support patch, filefield's "Rename $file['remove'] to $file['delete']" patch, and imagefield's database consistency patch. (Those patches partly conflict with each this one, I'll reroll concerned patches once any of them is applied.)

This patch applies to the DRUPAL-5--2 branch of imagefield, and should not be committed before all of the mentioned patch dependencies are committed themselves.

jpetso’s picture

Status: Reviewed & tested by the community » Needs review

Oh my god, I not only included a typo but also set the status field to the wrong status. Sorry... at least the latter can be corrected with this comment.

jpetso’s picture

StatusFileSize
new12.64 KB

Here's a new version, removing imagefield's hook_file_download() implementation. Filefield provides an exact copy of this function, and as imagefield now (with this patch) depends on filefield, we don't need to competing, duplicate versions of the same function.

jpetso’s picture

StatusFileSize
new12.64 KB

Update in order to work together with filefield-revisions-try8.patch or higher, as filefield_hide_upload_files() is now filefield_hide_upload_files_form(). No changes other than this renaming.

recidive’s picture

Patch looks good, I have not tested this yet.

For consistence I think it is good to use module_invoke() instead of calling filefield functions directly.

jpetso’s picture

I disagree with using module_invoke(); it should be used when any other module (also multiple ones) can hook into some operation. But that's not the case here, imagefield needs exactly these filefield functions to work.

In the future, we might want to just plug into filefield as an "image enabler" and would thus use the module_invoke() hooks in filefield, but that's still far far away and would require a completely different approach to implementing imagefield's functionality.

jpetso’s picture

StatusFileSize
new13.12 KB

Rerolled the patch for the latest changes in the DRUPAL-5-2 branch. Of course, it's a pleasure for me to provide an updated patch after getting all those fine patches applied :D

jpetso’s picture

StatusFileSize
new13.26 KB

Another update for the patch: As this version of the module doesn't implement the file_download() hook, hook_perm() is not needed as well. Also, the patch finally doesn't conflict with the .info file's "version" property, because dww removed those :)

Even if it looks that this patch won't be applied, I'll still keep updating it when appropriate.

dopry’s picture

Status: Needs review » Closed (won't fix)

I don't think i'm going to apply this to imagefield. I don't want to make it depend on filefield just yet.. There is too much special case handling... i'd rather work on a filefield_image.module for handling images uploaded by filefield.

I'm just going to mark it won't fix so people will know not to expect it as an imagefield feature, but a great aventue of exploration for the adventurous.