After applying the patch you left for the Mediafield module (http://drupal.org/node/144674), i would get a critical fail when trying to edit any node with a mediafield on it.

Fatal error: Class 'getID3' not found in .../sites/all/modules/getid3/getid3.module on line 59

The problem is 2 fold, first 

/**
 * Loads the getID3 library once and returns whether it was successfully loaded.
 *  
 * @return
 *   Boolean indicating if the library was loaded 
 */
function getid3_load($display_warning = TRUE) {
  @(include_once(getid3_get_path() .'/getid3/getid3.php')) or _getid3_library_not_found($display_warning);
  $version = GETID3_VERSION;
  return !empty($version);
}

The line "return !empty($version);" will never return false (on my install of php at least) as even if GETID3_VERSION was not set by getid3.php it will be "GETID3_VERSION" and so never empty, a better check is "return ($version != 'GETID3_VERSION');"

Part 2 is that this variable returned from getid3_load() is never used by getid3_instance()

/**
 * Create and initialize an instance of getID3 class.
 */
function &getid3_instance() {
  getid3_load();
  $id3 = new getID3();
  $id3-> option_md5_data = true;
  $id3-> option_md5_data_source = true;
  $id3-> encoding = 'UTF-8';
  return $id3;
}

this should instead be 

/**
 * Create and initialize an instance of getID3 class.
 */
function &getid3_instance() {
  $id3 = null;
  if (getid3_load()) {
    $id3 = new getID3();
    $id3-> option_md5_data = true;
    $id3-> option_md5_data_source = true;
    $id3-> encoding = 'UTF-8';
  }
  return $id3;  
}

I've attached a patch to this effect.

CommentFileSizeAuthor
versioncheck.patch961 bytesa_c_m

Comments

a_c_m’s picture

a_c_m commented

oh also i'm going to still install your patch for MediaField but with a bit of extra logic which will become redundant once this patch (or similar) is installed, just FYI.

robloach’s picture

robloach
he/him
commented
Version: 5.x-1.1 » 6.x-1.x-dev
Status: Needs review » Fixed

http://drupal.org/cvs?commit=123697
http://drupal.org/cvs?commit=123696

If there's still a problem with it, please feel free to re-open the issue.

drewish’s picture

drewish commented

i'd have preferred to have backported the defined('GETID3_VERSION') fix i did in D6 for the first problem:

/**
 * Loads the getID3 library once and returns whether it was successfully loaded.
 *
 * @return
 *   Boolean indicating if the library was loaded
 */
function getid3_load($display_warning = TRUE) {
  $getid3_path = getid3_get_path();

  if (file_exists($getid3_path .'/getid3.php') && file_exists($getid3_path .'/write.php')) {
    // A little workaround for getID3 on Windows.
    if (!defined('GETID3_HELPERAPPSDIR')) {
      define('GETID3_HELPERAPPSDIR', realpath($getid3_path .'/../helperapps') .'/');
    }
    include_once($getid3_path .'/getid3.php');

    // Initialize getID3 tag-writing module. NOTE: Their wanky dependency setup
    // requires that this file must be included AFTER an instance of the getID3
    // class has been instantiated.
    $getid3 = new getID3;
    require_once($getid3_path .'/write.php');

    return defined('GETID3_VERSION');
  }
  else {
    drupal_set_message(t("The getid3() module cannot find the getID3 library used to read and write ID3 tags. The site administrator will need to verify that it is installed and then update the <a href='!admin-settings-audio-getid3'>settings</a>.", array('!admin-settings-audio-getid3' => url('admin/settings/getid3'))), 'error', FALSE);
    return FALSE;
  }
}

but the fix for the second problem is probably correct.

robloach’s picture

robloach
he/him
commented
Status: Fixed » Active

That sounds reasonable.

drewish’s picture

drewish commented
Version: 6.x-1.x-dev » 5.x-1.x-dev
Status: Active » Fixed

i think this is "fixed" in d6 so if there's still an issue it's in 5. i'm willing to let sleeping dogs lie (lye?, lay?).

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.