[D7] Media Recorder

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Hi kenianbei,

After install successfully this module. I tried to use and got some issues for you :)

1. when I allow anonymous can access Voice Recorder page, I got this message:

Notice: Undefined property: stdClass::$sid in media_recorder_process_file() (line 462 of <....>/sites/all/modules/media_recorder/media_recorder.module).

2. On authenticated user, it usually high-light the control of recorder. See on the attachment 01.jpg.
After that, I refresh page and click on setting button, I can not do anything. See on 02.jpg
(browser: Chrome Version 26.0.1410.43; OS: Ubuntu 12.04)

3. After record success, click "Save recording" and I got:

Notice: Undefined property: stdClass::$fid in media_recorder_add_validate() (line 249 of <...>/sites/all/modules/media_recorder/media_recorder.module).
Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load() (line 178 of <...>/includes/entity.inc).

This is great module. I like it.

Hi kenianbei

Manual review

• Use the hook_uninstall() to eliminate the variables media_recorder_width, media_recorder_height, media_recorder_timelimit, media_recorder_upload_directory, and media_recorder when you uninstall the module.
• It uses "page arguments" instead of arg() to pass parameters to the callback.

  <?php
  function media_recorder_menu() {
    $items = array();
    // Callback that process the php://input from Wami.swf.
    $items['media_recorder/record/%'] = array(
      'title' => 'Record',
      'description' => 'Record a video or audio file.',
      'page callback' => 'media_recorder_record',
      'page arguments' => array(2),
      'access callback' => 'media_recorder_access',
      'callback arguments' => array(1),
      'type' => MENU_CALLBACK,
    );
  }
  
  /**
   * Menu callback for recording a media file.
   */
  function media_recorder_record($filename) {
    global $user;
  }
  ?>
  

• You do not need to define a media_recorder_access function because by default access callback calls user_access function.
• "callback arguments" is not a valid key.

Look at you https://api.drupal.org/api/drupal/modules!system!system.api.php/function/hook_menu/7

I have a problem. I can't access page with link file/add/record". I get message "You are not authorized to access this page."
How can I fix this. Thanks!

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxkenianbei1533270git

I'm a robot and this is an automated message from Project Applications Scraper.

https://drupal.org/project/audiorecorderfield

This sounds like a feature that should live in the existing audiorecorderfield project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the audiorecorderfield issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

Yeah, I can see that a new project could make sense in this case. Just make sure to document it explicitly on your project page what is different to audiorecorderfield.

Set this back to "needs review" whenever you are ready again.

This is a manual review :-)

First I would like to point to the importance of this module. AudioRecorderField (that I use) can only record using the Nanogong applet (java) or the Soundcloud flash recorder (flash). So, this module (if working) could fulfil the growing needs of HTML5 media recording.

Documentation issues

1. The documentation indicate the required modules, but it miss to point out the ctools dependency
2. From the documentation it's not clear which versions of wami library should be required. More in general, a higher level of details should be given on which exact versions of the libraries are supported.

Usage issues

1. After having downloaded the module on a fresh Drupal installation, together with the latest versions of modules and required libraries (as of today 28/6/2013), I can't get the recording widget working in either windows(ie|firefox|chrome) or linux(firefox|chrome). Please find attached a screenshot of the module configuration page.
2. In the same configuration page I can't get the description for the fields nor the fields that relate to the path.

Code issues

1. You miss to include in media_recorder.info a statement like configure = admin/config/media/mediarecorder so that the "configure" link is placed aside of the module in the modules list page
2. Same for the "help" link: you should implement hook_help() in order to have a help link in /admin/modules. For example, if you want just to point a link to the README.txt, you could use:

/**
 * Display help and module information
 * Implements hook_help().
 * @param path which path of the site we're displaying help
 * @param arg array that holds the current path as would be returned from arg() function
 * @return help text for the path
 */
function media_recorder_help($path, $arg) {
  if ($path == 'admin/help#media_recorder') {
    $output = file_get_contents(drupal_get_path('module', 'media_recorder') . '/README.txt');
    return nl2br($output);
  }
}

Hi, I let the "needs review" tag as I think now it's a documentation-only issue.. as I coudn't get the browser working in the environments you pointed ---
I am particularly dump, but I guess the average Drupal user is more or less at my level so I expect you will have lot of support request :-) What you need to do in a clean drupal installation to get it working? For example Modernizr needs to get its own library, and you haven't mentioned it in the installation steps (then of course it's up to Modernizr module to give more details, but you should at least mention it in the installation steps).

Wami:
- which of the "right click to download" file you need to put on sites/all/libraries/wami/ ? Just "recorder.js" or also the other files ?
SWFObject:
- swfobject_2_2.zip is fine?
Recorderjs:
- just recorder.js file (no "recorderWorker.js") ?

Hi

This a very interesting project! I'll look forward for it to evolve for video support.

Manual review:

• media_recorder.module, line 160, 191: unused global variable $user - just a super minor issue.
• media_recorder.module, line 263: there's a hardcoded path. Can you please explain me what the $filepath stands for? Wouldn't it be better to provide a corresponding setting for this?
  

    $filepath = 'public://media_recorder';
  

• media_recorder.module, line 396: wouldn't it be better to create a template for the youtube recorder instead of putting it's layout as #markup?
• media_recorder.module, line 471: text should be wrapped with t() function.

Proposals:

• It will be great to have a drush command that installs all needed libraries.

It will be great to put info about browser support to the project page.

Regards

manual review:

1. template_preprocess_media_recorder(): why do you need to call drupal_render() here? The render array is fine and should be rendered later, right?
2. media_recorder_record(): drupal_json_output() sues echo itself, so no nee for echo here.
3. media_recorder_add(): is the field name hardcoded here? Why? Please add a comment.
4. media_recorder_field_widget_form_record_validate(): could you use entity_label() for $title somehow?
5. media_recorder_youtube.js: all user facing text must run through Drupal.t() for translation.

But that are not application blockers, otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to tim.plunkett as he might have time to take a final look at this.

Trying to remove tag again.

no objections for more than a week, so ...

Thanks for your contribution, kenianbei!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.