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.

Files: 
CommentFileSizeAuthor
#37 1748952-audio-video-html5.patch12.64 KBDave Reid
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1748952-audio-video-html5.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#37 interdiff.txt12.62 KBDave Reid
#36 interdiff.txt667 bytesbrantwynn
#36 file_entity-audio_video_formatters-1748952-36.patch13.51 KBbrantwynn
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#33 file_entity-audio_video_formatters-1748952-33.patch13.28 KBsylus
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#30 file_entity-audio_video_formatters-1748952-30.patch13.32 KBgenjohnson
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#26 file_entity-audio_video_formatters-1748952-26.patch13.51 KBdouggreen
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#23 file_entity-audio_video_formatters-1748952-22.patch13.83 KBdouggreen
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file_entity-audio_video_formatters-1748952-22.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#21 file_entity-audio_video_formatters-1748952-21.patch28.09 KBdouggreen
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file_entity-audio_video_formatters-1748952-21.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 file_entity-audio_video_formatters-1748952-13.patch13.53 KBsylus
PASSED: [[SimpleTest]]: [MySQL] 308 pass(es).
[ View ]
#5 file_entity-audio_video_formatters-1748952-5.patch13.54 KBErik Erskine
PASSED: [[SimpleTest]]: [MySQL] 106 pass(es).
[ View ]
#3 file_entity-audio_video_formatters-1748952-3.patch7.89 KBErik Erskine
PASSED: [[SimpleTest]]: [MySQL] 106 pass(es).
[ View ]
#2 file_entity-audioformatter-1748952-2.patch4.55 KBErik Erskine
PASSED: [[SimpleTest]]: [MySQL] 106 pass(es).
[ View ]

Comments

Assigned:Unassigned» Erik Erskine

Status:Active» Needs review
StatusFileSize
new4.55 KB
PASSED: [[SimpleTest]]: [MySQL] 106 pass(es).
[ View ]

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

StatusFileSize
new7.89 KB
PASSED: [[SimpleTest]]: [MySQL] 106 pass(es).
[ View ]

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.

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

Status:Needs work» Needs review
StatusFileSize
new13.54 KB
PASSED: [[SimpleTest]]: [MySQL] 106 pass(es).
[ View ]

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.

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.

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.

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.

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.

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.

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.

StatusFileSize
new13.53 KB
PASSED: [[SimpleTest]]: [MySQL] 308 pass(es).
[ View ]

New patch against latest dev

Status:Needs work» Needs review

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.

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.

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.

Agree with aaron + slashrsm.

Status:Needs review» Reviewed & tested by the community

This looks great and works great.

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.

StatusFileSize
new28.09 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file_entity-audio_video_formatters-1748952-21.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled

Status:Reviewed & tested by the community» Needs work

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

StatusFileSize
new13.83 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file_entity-audio_video_formatters-1748952-22.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

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'.

Status:Needs work» Needs review

Status:Needs review» Needs work

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

StatusFileSize
new13.51 KB
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

Works fine for both audio & video

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.

StatusFileSize
new13.32 KB
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]

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

Status:Needs work» Needs review

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?

Status:Needs work» Needs review
StatusFileSize
new13.28 KB
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]

That was my bad :(

Attaching a patch without the associated div. My apologies!

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?

Status:Needs review» Needs work

Looks that way to me as well.

Assigned:Erik Erskine» Unassigned
Status:Needs work» Needs review
StatusFileSize
new13.51 KB
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
new667 bytes

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

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new12.62 KB
new12.64 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1748952-audio-video-html5.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

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.

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.