Hey folks,

I've been working on a patch that adds a checkbox to the field that allows for entering custom embed code, i.e. an iframe, to allow for non-support video host embedding. It's mostly functional, but there are some lingering issues.

1) (Major) Does not work when VEF is marked as required. I tried determining this in hook_field_widget_form(), but if the user changes the checkbox and then saves, it still thinks the previously required field is still required. This needs to be determined after submitting.

2) (Minor) When a node is first published, the embedded video doesn't show up. Every subsequent visit, or after a refresh, will properly show the embedded video. I haven't looked much in to this.

3) (?) The field is not validated in any way. Since it's just plain HTML, users could potentially put anything in here. Not sure how big of an issue this is.

Attached is a patch that has my work thus far.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

adrianedge’s picture

I have been trying to add this code to my video_embed_field.module file to use it even in its raw state. But I am not sure how to add and how to subtract and what lines up with what. Can you give me any clarification.

Best,

Adrian

adrianedge’s picture

This is a copy of my video embed field module file, where I was trying to apply your changes. I didn't get too far, but I'd like to implement this feature even with the bugs you describe so badly that I will keep trying.

I am guessing that the lines with the plus sign indicate code to be added, and the ones with the minus sign code to be taken away?? Am I correct?

There are some other symbols in front of some of the function calls, are these supposed to be there. I am viewing your code by clicking on it and viewing it in my browser (firefox).

Lastly, please help me update my file. I will be happy to compensate for the hours if you want.

Best,

Adrian

adrianedge’s picture

This is a copy of my video embed field module file, where I was trying to apply your changes. I didn't get too far, but I'd like to implement this feature even with the bugs you describe so badly that I will keep trying.

I am guessing that the lines with the plus sign indicate code to be added, and the ones with the minus sign code to be taken away?? Am I correct?

There are some other symbols in front of some of the function calls, are these supposed to be there. I am viewing your code by clicking on it and viewing it in my browser (firefox).

Lastly, please help me update my file. I will be happy to compensate for the hours if you want.

Best,

Adrian

jec006’s picture

Hi adrian,

The best way to apply his patch and get this working before its committed is to use git
If you go into the video_embed_field directory on a linux/unix machine/server and run:

wget http://drupal.org/files/embed-code-WIP.patch
git apply -v embed-code-WIP.patch

it should apply the patch for you (basically add the lines with + and remove the lines with minus).

If you don't have git on your server you can try something like

wget http://drupal.org/files/embed-code-WIP.patch
patch -p1 < embed-code-WIP.patch

but thats a bit trickier.

It is definitely my intention to get this into the module as soon as it is working consistently, so hopefully it won't be too long of a delay.

Hope that helps

jdelaune’s picture

Not sure the reasoning behind this patch.

Why not juts add a plain text field if you just want to use custom embed codes? You loose basically all the benefits of VEF by pasting in raw embed code. Including style settings, images etc.

I think the handler model is powerful and I would question why you don't write custom handlers to support the other sources you want to support. I write custom handers for the custom places our sites get videos from and you obtain all the benefits of VEF mentioned above.

jec006’s picture

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

I think that is a good point ... I guess to me the single benefit would be for view mode configuration - having a single field simplifies things. I think you're right though, having the ability to make it custom does ruin your ability to use the good stuff vef provides ... and losing the consistency makes using thumbnails for listings and such things not reasonable.

Alright, good call. We'll hold off on this for now. I think jdelaune is right, if you want this functionality, the best answer is to force embed of all things using a text field (youtube and vimeo will give you the embed code also).

thank you as always for the advice and points jdelaune :)

adrianedge’s picture

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

Thank you ! I am using a shared unix hosting package at Network Solutions, which doesn't allow for me to use GIT. So I will try download my site to a Xamp localhost environment on a windows machine, and use patch -p1 < embed-code-WIP.patch in Cygwin.

My second question is will the patched video embed module allow me to embed Ustream, and configure the space as it does for YouTube?

Secondly what do I need to know well to start coding and contributing to Drupal developments. I'm assuming a healthy knowledge of PHP is a good start ? What other systems?

Best,

Adrian

adrianedge’s picture

I read just the exchange before my last post.

Are you explaining that if I was to patch this module and then try to embed Ustream code in the Embedded Video field that I would loose the formatting and styling that a YouTube embed has once configured?

If so how do I work around this to be able to embed Ustream and maintain the look and functionality of embedding YouTube?

I've already tried to get this done by changing the arrangement in manage fields and using a full HTML text field to embed Ustream, but I loose all the formatting (including the voting, and description space) that's nice about this module.

Lastly how would one go about updating the handlers to include Ustream? And if not this Module, is there another that does this?

Best,

Adrian

jec006’s picture

Hey Adrian,

No, thats not the case, you just can't depend on the stuff being there across content items if custom embeds are allowed. I think in general this isn't really the route we want to take in vef (i know this is a reversal, but I thought about it more, and jdelaune is right). This definitely doesn't stop you from using the patch. The custom version doesn't' support thumbnails or styles though, so keep that in mind.

In re Ustream, ideally, you would create a custom handler for Ustream using the hooks provided by video embed field - you could then provide styles and a thumbnail yourself. Basically, you provide a couple functions that tell video embed how to find the video and the thumbnail and what options are available.

You can use the handlers in the module for youtube and vimeo as an example and there is documentation in the .api.php files in the module folder.

In terms of doing drupal development, I would say php is the main thing, and then just begin familiarizing yourself with the way that drupal works. Its a huge system with lots of its own standards, styles and ways of doing things.

adrianedge’s picture

I tried the patch after installing Cygwin. This is what I got.

/cygdrive/c/xampp/htdocs/LiveIslands/sites/all/modu les/video_embed_field
$ patch -p1 < embed-code-WIP.patch
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/video_embed_field.field.inc b/video_embed_field.field.inc
|index 22418e7..5a6daaa 100644
|--- a/video_embed_field.field.inc
|+++ b/video_embed_field.field.inc
--------------------------
File to patch: video_embed_field.module
patching file video_embed_field.module
Hunk #1 succeeded at 76 (offset 20 lines).
Hunk #2 FAILED at 94.
Hunk #3 succeeded at 111 (offset 21 lines).
Hunk #4 succeeded at 153 (offset 21 lines).
Hunk #5 FAILED at 162.
Hunk #6 succeeded at 195 with fuzz 1 (offset -17 lines).
Hunk #7 FAILED at 346.
3 out of 7 hunks FAILED -- saving rejects to file video_embed_field.module.rej
patching file video_embed_field.install
Hunk #1 succeeded at 24 (offset -3 lines).
Hunk #2 FAILED at 191.
1 out of 2 hunks FAILED -- saving rejects to file video_embed_field.install.rej

That course of action seemed to have failed so I have since abandoned that approach and have been trying to pick my way through creating custom handlers.

But I am very new to this and having very slow unsure limited or no success. It's been a huge learning curve today, through Cygwin, Patching and now updating the custom handlers. Can I PayPal you some compensation to get help with enabling this for Ustream or at least an example?

Best,

Adrian

adrianedge’s picture

Thank you for everything jec006. My last questions were a bit hasty. I've persevered through and realized that I was doing a number of things wrong, including trying to patch the wrong file, after taking a better look at the files in the packaged module.

Instead of patching however... I've edited the handles, and got a working handle for Ustream.

One problem.. Drupal forces me to uninstall video embed first to reinstall/update the video embed module ( with replaced updated files which I copied into the video embed module install package).

During the uninstall I have to destroy all the field attachments for the Video Embed module that come configured in my template, to get video embed to uninstall. My only option after doing this has been to reconfigure the video embed content type, but I have yet to get the theme reconfigured properly, with the same spacing. As a result my video screens are in the middle of the page.

Question... How do I find the video_embed_field.handlers.inc and other *.inc files, to replace them with my updated versions, instead of uninstalling the module, and destroying the fields. Can you please point me the *.inc files in the installed drupal module, or to where the code is embedded?

I've attached a screen shot of my spacing problem in case you have any other ideas how to fix it with another approach.

Thanks again,

Adrian

jec006’s picture

Hey Adrian,

I'm sorry for the delay, it's been a really crazy week.

The best way to go about adding a new handler to video_embed_field is to create a new module and implement hook_video_embed_handler_info.
This would look basically like this:

/**
 *  Tell video_embed_field about other handlers
 *  It should return an array of handlers, each handler is an array with the following keys
 *  'title' : required, the untranslated title of the provider, to show to the admin user
 *  'function' : required, the function used to generate the embed code
 *  'thumbnail_function' : optional, the function used to provide the thumbnail for a video
 *  'form' : required, the function that returns the settings form for your provider
 *  'domains' : required, an array of domains to match against, this is used to know which provider to use
 *  'defaults' : default values for each setting made configurable in your form function
 *
 *  @see below for function definitions
 */
function your_module_video_embed_handler_info() {
  $handlers = array();

  $handlers['ustream'] = array(
    'title' => 'Youtube',
    'function' => 'your_module_handle_ustream',
    'thumbnail_function' => 'your_module_handle_ustream_thumbnail',
    'form' => 'your_module_handler_ustream_form',
    'domains' => array(
      'ustream.com',
    ),
    'defaults' => array(
      'width' => 640,
      'height' => 360,
    ),
  );

  return $handlers;
}

Once you have told video_embed_field about each of your new handlers, you need to implement the callback functions that were defined for your
provider. They look like this:

/**
 *  Generate the embed code for a video
 *  @param $url - the video url as entered by the user
 *  @param $settings - the settings for this provider as defined in the form function, defaulting to your provider's defaults
 *  @return the embed code as a renderable array
 */
 function your_module_handle_ustream($url, $settings) {
   return array(
     '#markup'  => '<iframe src="ustream"></iframe>', //this should be the full embed code for your provider, including each of the settings 
   );
 }

 /**
  *  Retrieve information about the thumbnail for a given url
  *  @param $url - the url of the video as entered by the user
  *  @return an array with the keys:
  *    'id' => an id for the video which is unique to your provider, used for naming the cached thumbnail file
  *    'url' => the url to retrieve the thumbnail from 
  */
function your_module_handle_ustream_thumbnail ($url) {
  return array(
    'id' => '12332243242', //generally the id that the provider uses for the video
    'url' => 'http://something/thumbnail/somthing.jpg', //the url of the thumbnail
  );
}

/**
 *  A forms api callback, returns the settings form for the provider
 *  @param $defaults - default/current values for your provider, a mix of the currently saved settings and the defaults provided in info hook
 *  @return a form as defined by forms api
 *
 *  @see http://api.drupal.org/api/drupal/developer!topics!forms_api_reference.html/7
 */
function your_module_handler_ustream_form($defaults) {
  $form = array();

  $form['width'] = array(
    '#type' => 'textfield',
    '#title' => t('Player Width'),
    '#description' => t('The width of the player in pixels'),
    '#default_value' => $defaults['width'],
  );

  return $form;
}

Obviously, these are just skeleton functions and should only be taken as a template for building your module, but hopefully that gives you a slightly better idea of what you need to do. I think you probably have most of this as you have a player going, but I figured I'd write it all up so I can put it on the readme or whatever.

As for the player being below where it should be, that seems strange, is it possible that you have an unclosed tag in what your returning as the markup for the handler?

Hope that helps

jec006’s picture

I've also added this documentation to video_embed_field.api.php

jec006’s picture

Status: Needs work » Closed (works as designed)