Adding video.

I'm also having the same problem,
one more thing is while sharing the video , an error is comming like "Only files with the following extensions are allowed: jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp"

I have not looked into this but I suspect that video handling is currently broken.

Mainly this needs a developer's time. The original maintainer (publicmind) is unable to continue contributing to the project and so mathankumar and I are trying to handle some of the burden until we can get someone up to speed as the primary maintainer (Dinesh Kumar has agreed to do it but he has not had a chance to work on many issues yet it seems.) So I would not expect this to be completed in the short term unless you can find another developer to contribute a patch. Medium-to-longer term we should be back up and running more consistently on this project but it's a transition period right now.

I have also looked in to that issue. But It seems that most of the Drupal 6 modules like "emvideo", "emaudio" are discontinued. I will look in that issue this weekend and give a more update.

As of now my priority is making a stable release for Statuses and Modr8. If we are done with Statuses, then I will certainly work on this.

This weekend I planned to put my effort on both Statuses and Modr8.

I will help Dinesh kumar on getting started with porting these plugins.

Dinesh: thank you. My guess is Media is the likely D7 replacement but this does need additional research and consideration.

It seems that swftools are required for this video functionality to play videos.

This patch needs a lot of work. Its not a complete patch.
Its just a quick patch with the following modifications,
1. File validations for Document and video
2. Theme functions with appropriate arguments in D7 style.
3. Query modifications in D7 dynamic query style.

questions:
1. For video player to run it needs swftools module which is not available for D6. I am not sure how to handle that.. ?
2. Links uses emvideo and emaudio to detect an embed video and audio link, alternative suggested in media. a new issue for it http://drupal.org/node/1615178

Please verify the patch and post on ideas for alternative for swftools

Removed white spaces from patch

Thank you for the patch; everything in it looks good. Because you have mathankumarc's recommendation I went ahead and gave you commit access so you can commit it when you're ready (or wait until solutions to the remaining problems are incorporated into a new patch). Please make sure every commit you make is tied to an issue in the queue, and please make sure your patches get reviewed before committing for awhile until you get more familiar with the code base. (I will try to review every patch during this time -- if neither mathankumarc nor I reviews it after 1-2 weeks you can go ahead and commit anyway if it's tested or trivial.)

For video player to run it needs swftools module which is not available for D6. I am not sure how to handle that.. ?

I think that the only place FBSMP actually uses swftools is a single call to swf(). Honestly I am not really sure why the dependency is there. I don't know what the D7 alternatives are or whether an alternative is necessary at all, although it looks like SWF Embed does something similar. I'm sorry I can't offer a more specific answer -- I don't know a lot about how the actual file handling works. I reviewed all the code when it was written (this was a Google Summer of Code 2010 project and I was the mentor) but didn't have much input into that part and haven't spent much time on it since. So if there is a way you would particularly like to do it, feel free to just do it.

Thanks Isaac :)

Congrats Dinesh! Keep up the good work.

I think that the only place FBSMP actually uses swftools is a single call to swf(). Honestly I am not really sure why the dependency is there. I don't know what the D7 alternatives are or whether an alternative is necessary at all, although it looks like SWF Embed does something similar. I'm sorry I can't offer a more specific answer -- I don't know a lot about how the actual file handling works. I reviewed all the code when it was written (this was a Google Summer of Code 2010 project and I was the mentor) but didn't have much input into that part and haven't spent much time on it since. So if there is a way you would particularly like to do it, feel free to just do it.

I will help Dinesh on this, since we are colleagues we can collaborate better :)

Thank you Isaac and mathan..
I will go on my analyses with SWF Embed.
I will maintain separate Issue for features/issues.

Swfembed needs a flash object .swf to create the video. I looked around to grab any flashobject (*.swf) to use it. But will that be a Good solution ? or can we go with any specific player like jwplayer??

Removing the Dependency with swftools for video plugin and Added FlowPlayer Support to play Flv videos.

The following patch will Do that. But I am not sure what to do with thumbnails creation ?

quick review

+  if (module_exists('flowplayer')) {
+    $config = array(
+      'clip' => array(
+        'url' => $video_url,
+        'autoPlay' => $settings['autoplay'], // Turn autoplay off
       ),
-      'othervars' => $othervars,
     );
-    $output = swf($video_url, $options);
-  }

Check the existence of the autoplay value and set default value if its not there, else it will cause undefined error also change the comment.

+    $attributes = array('style' => 
+        'width:' . $settings['player_width'] . 'px;' . 
+        'height:' . $settings['player_width'] . 'px;'
+    );
+    $output = theme('flowplayer',  array('config' => $config,'id' => md5($video_url), 'attributes' => $attributes));
+   }

trailing white spaces. dreditor is not working properly on my browser, so cant provide exact code examples :(

I couldn't try this one now, will provide the complete review on tomorrow.

But I am not sure what to do with thumbnails creation ?

Me neither not looked into it, however we need to fix the thumbnails as part of this issue(if its not working)

Swfembed needs a flash object .swf to create the video. I looked around to grab any flashobject (*.swf) to use it. But will that be a Good solution ? or can we go with any specific player like jwplayer??

I honestly don't know. It looks like swf() from SWFtools was there to take care of that but I am not sure how to replace it. (As you've probably noticed I had very little to do with the actual saving and loading of files...)

I am not sure what to do with thumbnails creation ?

What do you mean? It looks to me like that should still work, courtesy of the FFMPEG library that FBSMP already provides.

I also haven't tried the patch yet, but other than the comments below and what @mathankumarc has already said, I don't have any complaints. (You should really use Dreditor if you don't already -- possibly also with this patch -- which will help you catch the trailing spaces and review other patches.) My general philosophy about patches like this one is that the module is already broken, so you can't really break it more :-) so if you want to go ahead and commit intermediate changes once they're reviewed and fixed that's fine (although maybe we should tag an unstable release first).

+++ b/plugins/video.inc
@@ -523,7 +523,12 @@ function fbsmp_video_fbsmp_delete($status) {
+  $video =  db_select('fbsmp_videos' , 'fv')
+   ->fields('fv')
+   ->condition('fid', $attachment->data['fid'])
+   ->execute()
+   ->fetchObject();

All indentations should be 2 spaces (there is just one here).

+++ b/plugins/video.inc
@@ -1082,7 +1132,11 @@ function theme_fbsmp_video_play_theora($video_url, $thumbnail_url = '', $setting
+   // @todo to change this something neat
+  $video_url = $variables['video_url'];
+  $thumbnail_url = $variables['thumbnail_url'];
+  $settings = $variables['settings'];

What were you thinking would be a better way to do this? It looks fine to me. Perhaps extract()?

@mathan and @issac

Thanks for the review. I will fix the patch.

What do you mean? It looks to me like that should still work, courtesy of the FFMPEG library that FBSMP already provides.

I mean thumbnails are very well created by FFMPEG library. Previously we used swf() to pass thumbnails created. But now as per comment #16 we support flowplayer. Which does not require thumbnails, as it displays the player itself.

I will also consider supporting video.js as mentioned here , through a configuration.

The Modified patch according to review by mathan and issac.
Pls review the patch.

The thumbnails are created and not saved.Need to work so keeping the ticket as needs work. Also take a look at http://drupal.org/node/1643422

Haven't tested, only one thing pops out at me:

+++ b/plugins/video.inc
@@ -992,28 +1023,31 @@ function theme_fbsmp_video_play_dcr($video_url, $thumbnail_url = '', $settings =
+    $auto_play = isset($settings['autoplay'])?$settings['autoplay']:0;

There should always be a space around operators, e.g. condition ? true : false

I wish there was an easy way to see interdiffs.

@issac Thank you and sorry for the delay.

I have modified and also fixed on Video conversion and Thumbnail creation of the videos (Thumbnails are not created) with the patch attached.

Pls do review that

Will try to review this weekend

Changing the status.

I honestly don't have a sense for whether the file-handling stuff is logically correct, but I don't have any objections to the patch. Leaving at "needs review" because I haven't tested it. If it works, let's commit it.

Just now tested this one, its working perfectly fine with one exception. While adding the video with using ajax, the video is displayed after page refresh only. I think we can handle this in a separate issue.

@Isaac Can we make an unstable release after this commit?

@mathan thank you,

Committed to dev

Awesome. Yeah, go for it!

EDIT: Leaving open until a follow-up issue is created

This is the follow up issue for video player not playing in ajax call.
#1706206: On adding video in ajax, the video player is displayed after page refresh
Closing this as fixed.