Video should play on the same page

This patch runs the body through the theme, and the theme adds the functionality of the video_play function. This gives me a video on the view page that starts playing immediately.

The major thing that I'm unsure of is whether this limitation in the workflow was introduced because of previous versions of the module or because of limitations with the browsers being able to handle some of the file types for some reason. So far this has only been testing with flvs, so feedback is welcome.

This patch should probably also has a setting in the video settings page to NOT play the video in the page for those who want to use the view -> play method.

After thinking about this a bit more, does it make sense to pull that case statement into it's own centralized function so that both video_play and theme_video_view can call it to get the html for the video player. if so, I'll try and put this together later today.

Here's an alternate version that centralizes the video-type case statement. This new theme function then gets called by both the video_play funciton and the video_view's theme function. This should make it easier to add new video formats.

Note that in video_view I also wrapped the theming of $node->body in a check for $page.

I realized that this also creates an issue for video_image where you would get a page with the image and the video next to each other. It should be a simple fix to not display the image in the body if you are already displaying the video, but as I'm not sure what direction the module maintainers want to take video module, I'll let you guys take it from here.

Looks like I missed the "play video" access permission in theme_video_theme. This version includes that check. I promise this is the last follow-up for the day.

really a good work. this is a needed feature.

but we can't change the implementation without let users decide
if continue using the play page or play on view.

so please add a setting for let users enable this optional feature.

then I will commit this.

Thanks again.

Fabio

Good point.

I’ll try to put something together in the next day or so.

I’ll also try to open a new issue and upload a patch that deals with the video_image problem I mentioned above.

Here is a new version that gives admins the option whether to put the video in the body of the node or not. It creates a variable from the settings page, which is checked in the new theme_video_view function.

In order to keep this patch as simple as possible, I left the current configuration options as the default. Someone may want to roll up an additional patch that cleans up some of the options in the settings page and changes the default behavior.

Note that I also went ahead and updated the patch to work on version 1.48, as opposed to v1.47

Good work on the patch.

However there are still some issues.
The patch does not change the tabs behaviour.
As I'm playing on the view page there should not be a play tab.

Moreover the play link should point to node/XX not node/XX/play

please also note that the current video module version is 1.51
(solved some little bugs - see cvs log for details)

Does this argue for a redesign of the settings options then? As I said above, I wanted to keep the patch as simple as possible, and revamping the settings page seemed a bit much at the time. I agree that with the new "play in node" option, it is possible to make some choices that don't really make sense. I'm not sure I have a good grasp on the current user needs, but I'll take a crack at it.

What if we condensed the current top two fieldsets into one that looks something like this:

Playback options (radio option group)
Play in node
Play in tab
Play via play link
Play via play link and tab

Download options (radio option group)
Don't allow downloads
Display download tab
Display download link
Display download link and tab

Additional information options (checkboxes)
display playtime
display filesize

Thoughts?

After thinking it over, it seems out of focus to try and redesign the settings page in this patch, so instead I have tried to limit the available options. To that end, this patch includes a video_settings_form_validate function that will throw an error if the user has "Play in node" set to "Yes" and either "Display play menu tab" or "Display play link" set to "Yes." While this might not be optimal, it keeps admins from choosing configurations that don't make sense.

The idea here is that the play tab and the play link are redundant and result in a bad user experience. While I could see that it is theoretically possible for someone out there to want all options, this minor limit on functionality keeps the patch light, while also keeping admins from accidentally creating a false impression that the site is broken.

Additionally, changing the settings and moving around the variables could cause unexpected results when upgrading old installs of this module.

I'd suggest that we start a new issue to redeisgn the settings page with a proper update path in the install file.

At any rate, here is the patch for review, ready for version 1.51 of the module.

Updated version of the previous patch. This adds a check to make sure the admin selected at least one of the playback options. Since there may be a need for people to use an alternate method (via a theme for instance) to display videos, I have set this to a drupal message instead of a form error message that would keep the admin from submitting.

Feel free to adjust the wording, etc.

Once again, if someone wants to take on a settings page redesign after this patch, that might be a good idea.

This is patch is really useful! Great!
but i have a question. How can play videos also in "teaser" and not only in "body"?

could someone post the video module pre-patched? I know I would need to learn eventually.. but not right now.

any help would be great

you could patch at your own. it's easy

+ add the line
- delete the line
:D

Psicomante: I don't think most people would want the video in the teaser as this could quickly turn into your typical myspace page where 10 videos start playing at the same time. We are using a patch with the video image module (http://drupal.org/node/81157) that we rolled up to better support the play in node display. However, you make a good point that some people might have a situation where they do want to show the video in the teaser, and to that end I have adjusted the patch.

The function video_view() now passes the teaser through a new theme function called theme_video_teaser().

neon_eddy: My suggestion would be to install eclipse and the phpeclipse add on. It makes dealing with cvs and patches a breeze once you get it set up. Otherwise, it sounds like we are getting close, so you might be able to wait until Fabio gives it the green light and commits it.

Fabio: Will this patch make it in for the 4.7 branch considering the 5.0 post above?

Here is the actual patch that goes with the post above.

This patch and the new feature will be included in the next 5.0 video module release.

The patch changes the behaviour of the module so it is not possible to modify it
in an already deployed version.

Ok... The patch is a good step forward in usability.

There are still some minor issues:

user acces logic (user_access('play video')) should not be in a theme function.
please move it to a non theme function (video_view ?).

If user does not have permission to play the video a message should be printed
to advise him of the insufficient permissions.

I think that the "Play in node" and "Display play menu tab" should be grouped in
the same radios element. Maybe something like "Play behaviour" where there are
two radios: play in node and play on a separate tab.

But as you already noted above a modification of the setting page is needed at this point.
Feel free to open an issue for this.

Good points.

I'm pretty snowed under and I probably don't have time right now to bring this up to the 5.0 version. If someone else wants to take a crack at updating the patch and fixing the issues you mentioned, that'd be great. Otherwise I'll try and come back after I get past some other projects I'm working on. Thanks!

Will this feature be available for the 4.7 version? This and the auto flash conversion are very high on the list of user requests at the moment

No, 4.7 will get only bugfixes. This will be the default behaviour of video module 5.

However you can apply this patch to your 4.7 video module.

Fabio

Apologies if this is a stupid question! I'm new drupal but would love to get this patch to work on my site (4.7). How do I install the patch? Do I need to add the lines that have + and take out the ones marked - or do I simply add this into my video.module file somewhere? I was going to add the + lines and take out the - lines but wasn't sure what the other lines meant in the patch, like @@ -284,7 +290,7 @@.

Thanks for any help.

just apply one of the earlier patches above which were 4.7 based.

Thanks. Do i add the + lines and take out the - lines? Also what do I do with the lines such as @@ -284,7 +290,7 @@? I'm just not sure whether to paste the whole patch into my existing video.module.

Thanks again.

Ah, figured it out, I didn't realize i'd need a patching program. Great patch by the way, just what I was looking for. Thanks

This is SWELL, thank you so much for the work, folks. Greatly needed for sites that can't make the leap to 5.0 (& Fabio's upcoming integrated solution) just yet.

But: video display in node only works when I'm at /?q=node/x. It doesn't work using Views ('full node' of Type:video selected) or php block snippets that display the full node. I just get the title which links to the full node. I have teasers turned completely off, but from reading this thread assume that teasers are probably not the problem? Is there something that prevents the player from displaying anywhere other than /?q=node/x ? Thanks!

~~> for those who can't/don't know how to patch, or are on Macs with no ssh access via Terminal: attached is the patched version of video.module using the latest patch, video_play7.patch by Sean in comment#19. Use at own risk. Works for me on a spanking fresh 4.74 install.

see permissions.

Thanks, Fabio. Turns out it wasn't the permissions (I was viewing as admin and as auth user with full permissions) - it was the settings of Views itself.

Selecting the dropdown 'Full Nodes' view (page or block) didn't work, as described above (I get title, no video). I've tested and this toggle in Views works with full content of other modules, but not video.module with the display-in-node patch from this thread. Not sure if this is specific to my install, something to fix or just take into account when using this patch. Noted for others, anyways.

Solution: I had to make it a 'List View' with the Node:Body field selected for filter Node Type:Video. Yay! This does exactly what I needed - I can choose only the Body field & get the pure video, nothing else, listed in my View.

Many many thanks to all, esp to Fabio for the fabioloso video module.

setting http://drupal.org/node/107874 as duplicate of this issue.

Fabio

Here is an updated patch for 4.7 for those who want this functionality

Thank you Grugnog2 for your work.

However this patch has still some problems making it not ready to be committed.
Please see http://drupal.org/node/76319#comment-138579

If those problems are solved this will get commited.

Thanks,

Fabio

Hi Fabio,

I wasn't intending for this to get committed - since according to http://drupal.org/node/76319#comment-148666 the 4.7 module is in bugfix mode, and the 5.0 already has this functionality. I agree the admin UI given by this patch is really quite confusing - but it doesn't seem worth the effort to fix if this isn't going to get committed ;)

I was just updating to patch so others could use it on 4.7 if they are not ready for a 5.0 upgrade yet.

Thanks!
- Owen

this feature is still not yet available on video module 5.0.
This is why I was asking for help in creating a committable patch.

Fabio

Okay I took the above patch, converted it to 5.0 and made the changes you wanted. ie not using user access control in theme funtion, and printing out if a user doesn't have permission to play a video.

Also I corrected a small logic error, where if display play link is off and someone didn't have permission to play a video it would display 'login register to play video'

Hope this is satisfactory and we can get this committed.

forgot to change status to "needs review"

This is maybe that what i am looking for but i dont know how to apply that patch :(

chazz: http://drupal.org/node/60108

fax8 have you had a chance to look at my latest patch for this?

I personally think we should remove the option of having the video play on a separate page, I can't imagine any scenario where that is useful.

I briefly looked at your patch... which seems a good work.
However I did not tested it.. I'm going to test this on the weekend.

I also agree that having the video play on a separate page is a not often used feature.
However I'm wondering about the upgrading process of already established video websites...

What does will happen to them when they upgrade to 5? The behavior of the video module
will be really different and I'm sure people will get into problems..

What are your thoughts about this?

Fabio

agilpwc, i just finish test on my drupal 5.1 and your patch is working fine.
That is what i am looking for. I have now the video (from youtube) into node page, not like before after click play button.
At the moment everything is fine i will try to test it again on the morning
Well done!

Committed to CVS.

Thanks to all who contributed to this patch.

Fabio

I was looking for an option similar to the origin post - display video overview (where the thumbnail is), and play the movie at the same page. Play in the body is excellent idea, but not quite what I'm looking for.

I believe the attached (zip file) patch provide a solution. It pertains to two files: video.module, and acidfree class_video.inc.

Environment: drupal 5.2, video 5.x-1.x-dev, Acidfree 5.x-1.x-dev.

Please disable page cache before commit changes.

Please disregard above two notes.
Attached please find version 02.