Big performance problem on dnd_library when loading videos

Seems like this issue affects scald audio as well.

I think the problem is with <video preload="auto|metadata|none">
The default value seems to be 'metadata' for firefox (which is correct IMHO), but 'auto' (that is "full") for webkit based browsers, which is critical for the sdl_preview context;
This setting should be a player option, but unfortunately, the default player cannot have options...
For a temporary fix, the template could be :
<video id="scald-video-<?php echo $vars['atom']->sid; ?>" controls="controls" preload="metadata" class=...

I think the problem is worse than that, as im getting this issue on a site where i have no qtip configured, that part of the js does not even run, so there should be no preview in the markup at all.

Found the issue :)

We have an image preload code in the js:
dnd-library.js
line 333

  var cached = $.data($(editor), 'dnd_preload') || {};
  for (var editor_id in Drupal.dnd.Atoms) {
    if (!cached[editor_id]) {
      var $representation = $(Drupal.dnd.Atoms[editor_id].editor);
      if ($representation.is('img') && $representation.get(0).src) {
        $representation.attr('src', $representation.get(0).src);
      } else {
        $('img', $representation).each(function() {
          $(this).attr('src', this.src);
        });
      }
    }
  }
  $.data($(editor), 'dnd_preload', cached);

The .editor contains the video tag or audio tag, and it will put that into a jQuery object, which basically preloads them. But we only wanted to preload images and not videos and audio files i suppose. So we need something smarter there.

Right now, the preview item is always added to the library JSON reprensentation, no matter what JS may be available JS wise (seen scald_dnd_library_add_item).

This code came straight from the D6 codebase, where there was no way to fetch additional data through another AJAX call, and thus the library response needed to include every possible bit of information. Since then, a lot of things happened, and maybe its time to revisit those previews a bit. There's probably potential for saving some server CPU cycles.

Notably, I think we probably could:

• Remove the preview from scald_dnd_library_add_item
• Tweak dnd-library to stop using {content: {text: Drupal.settings.Atoms[id].preview, and instead either use {content: {ajax: ( qTip2 only) or implement our own load callback through Drupal.dnd.fetchAtom, and only fetch the preview when it's actually needed

The drawback is that each preview will have to roundtrip to the server, so the preview will take a bit longer to appear ; it's also slightly less ideal for a user that would look at all the previews. I think that's an edge case though.

That being said, fixing the previews might not be enough to fix Webkit. The library response also includes atom rendered in the standard sdl_editor_representation context, that's supposed to be used as a default. I guess those players too might trigger the video autoloading… so we may also need to tweak video default player to prevent the browser from doing "crazy" things here. I think defaulting to only metadata fetch in the default player, and generally speaking enforcing sane defaults for the case of a bunch of videos embedded on the same page is fine ; it might be a good idea to add a second player to Scald Video, exposing a bunch of the attributes, named something lke "Advanced player". We probably could put autoplay / preload / muted / loop in that player options, and that way people who want to have a main video autoplay-ing, mute as first class media on top of their page can have a player suiting there need.

Thoughts welcome :-)

We have two problems here,
1. The issue with the image preload, preloading video and audio.
Working on a patch for that.

2. If qtip is enabled then the preview will load the videos and audios, but only IF the qtip is enabled. Otherwise they remain in their javascript object.

( #7 was a crosspost with @nagy.balint comment in #6, but it's basically the same ; .editor is the sdl_editor_representation I'm talking about in the paragraph above, and the reason for "put that into a jQuery object, which basically preloads them" is that jQuery will start creating DOM nodes for the preview, which will make browsers parse the video tag, and start preloading as instructed, even if the node isn't attached to the current document DOM tree)

This patch will solve the critical part.

Basically i replace video and audio tags out of the string before converting it to a jquery object, as that code was supposed to preload images only.

About the preview issue, it only appears if qtip is enabled, the question is what should happen if we view the preview of an atom, should we be able to play the video? Cause in some cases qtip can be configured to remain open, and in that case the client might like to press that button. So that part is not soo easy.

So if no objection. then i can commit this soon to fix the issue which happens even when qtip is off and preview is not loaded.

For the qtip issue, i guess we could use @gifad 's approach indeed, and add preload metadata to both the audio and video tags in both providers.

Then we can either add a custom atom attribute later, like preload type, and then the content editor can choose between preload type per placement, or we can have another player as @DeFr wrote.

This of course does not invalidate the other patch, because even the metadata should not be loaded if qtip is disabled and we are just image preloading.

According to this the preload attribute is not supported under internet explorer http://www.w3schools.com/tags/att_video_preload.asp
But the audio is supported: http://www.w3schools.com/tags/att_audio_preload.asp

Since there is no autoplay currently in the players, the user has to click on the play button anyways, so only preloading metadata does not seems to ruin UX.
I think we can safely default to metadata preload.

Later on of course we can have players or atom attributes to change this. Or of course users can always override the template and define different players for themselves.

Here is the combined patch.

@nagy.balint: Are you sure that the scald_video_player hunk is not enough to fix the whole thing ?

Well we need to add it to both the audio and video players as they act the same way. So that two is needed.

The other hunk in the preload is needed as we would like to avoid even downloading the metadata if we dont even display a player anywhere on the page, then its unnecessary download, even if its small, i think its better to not require the client to do those unnecessary requests.

The preload code is already supposed to only act on <img /> tags, forcing the preload by changing the src. For video and audio, I'm pretty sure the preload code as is has no effect ; I think it's only that there was no preload="metadata" in the player that made the browser starts downloading the video.

Now that the attribute is added in the templates, sending the whole .editor to jQuery should have no ill effect.

I did test it, and it did have queries without that.

Have you tested it?

So I removed my js snippet, and i see the following unnecessary queries:
big_buck_bunny.webm
Request Method:GET
Status Code:206 Partial Content
1.5 KB

big_buck_bunny.webm
Request Method:GET
Status Code:206 Partial Content
68.0 KB

Both of these I get twice in fact, and the same with audio.

I dont think these queries should be there, so my js snippet is required.

Well, those queries correspond to the metadata preloading that's requested in the preload attribute, so having them make sense :-) The resulting answer from the server shouldn't be bigger than an image, so if having those queries is a real problem, then the whole image preloading probably is too.

That javascript code there clearly states that it is doing image preloading. Preloading metadata for audio or video is not image preloading.
So either then we should remove the whole code part there (not sure why it is there in the first place), or if we keep it then it should do just that, preload only images. In my opinion.

But then likely removing the whole part is the best option, unless we have a good reason to keep it.

The reason this preloading is there is to warm up the browser cache to make the drag'n'drop experience more satisfying for the end user, having everything appearing instantly for the end user after a drop instead of waiting for the needed information to be fetched up from the server. Concretely, that means having the image in the browser cache for Images, and metadata to be able to render the player correctly (correct time length, …) for audio and video.

The nice side-effect that's image specific is that it also ensures that the image derivative is generated if needed on the library display, instead of waiting for the atom to be dragged into an RTE and then waiting for that stuff to be generated, which can take some time.

Fine.
Here is the updated patch.

Looks good I think. The only thing I'm still puzzled about is the fact that you're getting two queries, when obviously one would be better. Probably can be dealt with in a followup.

Moving to RTBC.

Note that the positive aspect of image preloading is not applicable when using the dndck4 plugin !
More so if the sdl_preview context uses the same transcoder as the sdl_editor_representation;
The negative aspect of slowing down the library navigation is sensible, though...

But if the ajax loads the editor representation in dndck4, wont that be an already preloaded image? so it should be faster?

@gifad: That should be applicable to DNDCK4 too, at least for all the fields were the default representation is the Editor representation.

Thanks, committed.

Hello there,

We're still experiencing a big performance hit. We are building a site that could potentially have thousands of audio and video files. And so this translates on a huge performance hit when going to any node/edit|add page; since it will try to pre-load them.

So we tested out solution in #10, but it introduced other issues when using the dnd functionality on rich-text fields using WYSIWYG+CKEditor.

This patch we're contributing addresses the same issue as from #10, but in a different way. The end result is the same: It will completely skip pre-loading any Audio|Video files.

We hope this help others in our same situation.

Regards,

Forgot to upload the patch. Here it is.