Add two new file field formatters (which then can be used as file formatters):
- HTML5 audio
- HTML5 video

These two field formatters should have the following options:
- Output field values as: [multiple audio tags|multiple sources]
- The rest of the options that audio and video tags support

We should also add two additional theme functions, like theme_file_audio() and theme_file_video() to do the work of the actual output.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

erik.erskine’s picture

Assigned: Unassigned » erik.erskine
erik.erskine’s picture

Status: Active » Needs review
FileSize
4.55 KB

Added an initial version of audio element formatter. Still to add support for more attributes, and a video formatter. Comments welcome

erik.erskine’s picture

Here is an extension of the patch in #2 that adds a video formatter.
These formatters also ignore any files that are not of the appropriate type.

slashrsm’s picture

Status: Needs review » Needs work

I installed it, tested both and and it basically works. The biggest problem I notices is with configuration for tags/sources. It seems like formatter uses tags (one file per tag) even if I selected "sources" option.

+++ b/file_entity.field.incundefined
@@ -113,5 +164,73 @@ function file_entity_field_formatter_view($entity_type, $entity, $field, $instan
+  else if ($display['type'] == 'file_audio') {
+    $multiple_file_behaviour = $settings['multiple_file_behaviour'];
+    $controls = isset($settings['controls']) ? $settings['controls'] : TRUE;

I think you should handle default settings for 'multiple_file_behaviour' here.

+++ b/file_entity.field.incundefined
@@ -51,6 +96,12 @@ function file_entity_field_formatter_settings_summary($field, $instance, $view_m
     $summary = t('View mode: %mode', array('%mode' => $view_mode_label));

It would be nice to display entire configuration in summary, not only one.

+++ b/file_entity.moduleundefined
@@ -10,7 +10,7 @@
-define('FILE_DEFAULT_ALLOWED_EXTENSIONS', 'jpg jpeg gif png txt doc docx xls xlsx pdf ppt pptx pps ppsx odt ods odp');
+define('FILE_DEFAULT_ALLOWED_EXTENSIONS', 'jpg jpeg gif png txt doc docx xls xlsx pdf ppt pptx pps ppsx odt ods odp m4a');

Why did you add m4a here? Was it intentionally or by a mistake?

It is also almost impossible for an ordinary user to start video/audio, if you decide to hide controls. Adding support for autoplay would help in this case. It would be nice to support all attributes of , and anyway (autoplay, preload, width, height, type (file's mimetype) ...).

http://www.w3schools.com/html5/tag_video.asp
http://www.w3schools.com/html5/tag_audio.asp
http://www.w3schools.com/html5/tag_source.asp

erik.erskine’s picture

Status: Needs work » Needs review
FileSize
13.54 KB

Thanks for the feedback, slashrsm.
Here is a new patch:
- test for settings that don't exist and provide suitable defaults
- I've added some more options - autoplay/loop for audio, autoplay/loop/muted/width/height for video.
- removed the new m4a extension - indeed that was a mistake.

RobW’s picture

Have you checked out http://drupal.org/project/html5_media yet? It seems like there could be a lot of overlap here, maybe some code that could be re-used.

Dave Reid’s picture

We are shooting for a much simpler alternative that html5_media that does not include a large library. We just want native audio and video tags as formatters/theme functions.

slashrsm’s picture

Status: Needs review » Needs work

Looks OK now. Thank you for the great patch.

I'd say that it would be nice to have tests for this. I am aware that there are not many in file_entity, but I think we should start adding them. When a new feature is added it is IMO a great time for that.

RobW’s picture

It would be nice to have the markup provided by tpls, which are generally easier for front end developers to work with. I'll see if I can add them sometime over the next two weeks.

quicksketch’s picture

It seems to me that this request is a bit out of scope for this project. I haven't had much involvement with it, but based on this module's description, I wouldn't expect File Entity module to be providing functionality beyond its stated purpose of making files fieldable. Especially considering HTML5 video and audio tags aren't universally supported, and Firefox's ongoing resistance to MP3 licensing, HTML5 audio and video tags just aren't universal enough to be useful for most sites. I would expect to have to install a module that provides an HTML5/Flash based player for either audio or video for the time being. If File Entity also provides formatters for video/audio, it's just going to make things confusing that we'll always have HTML5-only formatters that we'll need to ignore.

travist’s picture

I agree with quicksketch. If you don't have the fallbacks in place, then the solution is somewhat incomplete. I have done very similar work to this patch in the module http://drupal.org/project/html5_media as RobW pointed out (which I just updated with the latest version of media player), so you can definitely look at that as a reference if you would like. But as Dave pointed out, adding a huge library into the project may not be ideal.

Another alternative is to implement something similar to what Video For Everybody does where it includes an image placeholder inside of the tags if they are not supported. I am thinking that this may be best for this module since you do end up with some form of fallback (even though it sucks), but you are also not forcing people to download a large media player library with this module.

Devin Carlson’s picture

sylus’s picture

New patch against latest dev

Devin Carlson’s picture

Status: Needs work » Needs review
queenvictoria’s picture

Patch in #13 works well for me.

FWIW I think the Flash question (alternate rendering for browsers that don't support HTML5 Video/Audio tags) should be handled elsewhere. Its a layer on top of functional HTML (like animated menus for example) rather than core functionality. I guess that is progressive degradation rather than enhancement. In my opinion we should be providing some file display markup (as it becomes available) rather than rendering an "I don't know what to do with this file". Just my 2c.

aaron’s picture

I still waffle a bit on this issue but I agree in large part to what quicksketch says. On the other hand, it would be nice to have video and audio support out of the box. Additionally, this might help nudge html5 adoption, if only a very little bit. I wonder if we might consider moving this patch over to the media module.

slashrsm’s picture

I agree with aaron. I am convinced that we need to provide and formatters by default. Once you have that it is relatively easy to provide FLASH degradation (enable mediaelement.module and enable it site-wide).

I am voting for inclusion of this formatters in file_entity/media and writing some proper online docs on how to use them + list of modules that provide degradation.

sylus’s picture

Agree with aaron + slashrsm.

aaron’s picture

Status: Needs review » Reviewed & tested by the community

This looks great and works great.

Devin Carlson’s picture

While I'm +1 for including basic support for and in File entity, I'm not sure about adding it to 7.x.

Since Drupal 7 is XHTML+RDFa 1.0 out of the box are we assuming that, if they want to use the formatters, everyone is okay with either File entity generating invalid markup (and not working at all in older browsers, such as IE6-8 which are supported by Drupal Core) or running an HTML5 theme/changing html.tpl.php themselves?

I've got the same thoughts about leaving #1871792: Add support for mapping certain fields to HTML5 data- attributes out of File entity until 8.x.

Otherwise #13 looks great.

douggreen’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, file_entity-audio_video_formatters-1748952-21.patch, failed testing.

douggreen’s picture

I don't know why the patch failed testing, it's complaining about entity_file.module not existing, that seems like a failure on the testbot!?

Attached is a slightly updated patch, which includes {#2046467], and changes 'else if' to 'elseif'.

slashrsm’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, file_entity-audio_video_formatters-1748952-22.patch, failed testing.

douggreen’s picture

Devin Carlson’s picture

Status: Needs work » Needs review
mollyavalon’s picture

Status: Needs review » Reviewed & tested by the community

Works fine for both audio & video

Dave Reid’s picture

Status: Reviewed & tested by the community » Needs work

Is there any way to hide the 'multiple_file_behaviour' options when being configured as a file formatter as compared to a field formatter? It doesn't make much sense to show it in that case? If it's not possible, that's ok - let's just file it as a followup task.

genjohnson’s picture

I've changed the spelling of "behaviour" to "behavior" based on the coding standards.

Devin Carlson’s picture

Status: Needs work » Needs review
Dave Reid’s picture

Status: Needs review » Needs work
+++ b/file_entity.theme.incundefined
@@ -68,3 +68,91 @@ function theme_file_entity_download_link($variables) {
+  $output .= '<div class="wet-boew-multimedia span-4"><video' . drupal_attributes($video_attributes) . '>';

Hrm, guessing this <div> wasn't exactly meant to be in this patch?

sylus’s picture

Status: Needs work » Needs review
FileSize
13.28 KB

That was my bad :(

Attaching a patch without the associated div. My apologies!

Dave Reid’s picture

Has my question in comment #29 been answered?

Is there any way to hide the 'multiple_file_behaviour' options when being configured as a file formatter as compared to a field formatter? It doesn't make much sense to show it in that case? If it's not possible, that's ok - let's just file it as a followup task.

At first glance it doesn't look like the patch does anything to handle this yet?

aaron’s picture

Status: Needs review » Needs work

Looks that way to me as well.

saltednut’s picture

Assigned: erik.erskine » Unassigned
Status: Needs work » Needs review
FileSize
13.51 KB
667 bytes

I believe we can just disable this based around whether or not $field returns NULL on the settings forms. Patch and interdiff attached.

Dave Reid’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
12.62 KB
12.64 KB

I made some tweaks to #36 while testing:

  1. Default the behavior to output multiple tags and not multiple sources within one tag. Feel like it will be less confusing for people trying out the formatters on multiple value fields.
  2. Use #access directly on the multiple_file_behavior element
  3. Removed isset() on all the $settings values. We should be able to rely on them being available.
  4. Moved the height and width fields to their own elements like Image style configuration, rather than doing it inline. More compatible with the core themes and vertical tabs.
  5. Fixed the formatter summary had 'indivisble' <audio> and <video> tags in the summary itself.

I might follow-up with some cleanups to the 'multiple_file_behavior' handling and form elements, but this is good for now.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1748952-audio-video-html5.patch, failed testing.

Devin Carlson’s picture

Status: Needs work » Fixed

#37 failed testing because it has just been committed!

http://drupalcode.org/project/file_entity.git/commit/6cb80eb

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

Albert Volkman’s picture

Issue summary: View changes

Sorry to resurrect an old post... However, I see that this is a field formatter. Is there a form element as well so that I can use this in a custom form? Basically I'm assuming it'd be a video/audio field, set to unlimited? If there's another ticket for this request, please kindly point me in that direction. Thanks so much.