search

CenterForOpenScience / modular-file-renderer

A Python package for rendering files to HTML via an embeddable iframe
http://modular-file-renderer.readthedocs.io/en/latest/
Apache License 2.0
44 stars 67 forks source link

Set the poster and preload attributes for videos hosted on OSF #360

Open allsey87 opened 4 years ago

allsey87 commented 4 years ago

To save a lot of OSF bandwidth and to speed up the loading of content, I would strongly suggest the use of the poster=path/to/poster.png and preload="none" attributes of HTML video nodes. This would impact the video preview panel when browsing project files and also videos embedded in the wiki using the @[osf](GUID) method.

It would also be nice if eventually the poster images could be selected by the users when uploading video files, but I would say this is lower priority for now. At a glance, perhaps this could be achieved using export handler that generates a thumbnail at a user specified offset.

Johnetordoff commented 4 years ago

Yep confirming your other comment this line seems to be the one in snippet that needs changing.

As long as tests reflect that change, or there are new tests for this I personally would approve this, but again a reminder there's no guarantees it will be adopted anytime soon or at all, it has to go through a more thorough review process then just my say so and we are serious about QA testing every feature so that takes time and money.

Lastly just because it's so rare we actually accept these, if we test this and it there are problems that require changes we will not be able to wait for you to make edits and we will just take over the PR and make our own edits or just drop the issue. So I hate to dissuade someone from helping us out, but I'm trying to give a realistic view of what will happen if you do submit a PR.

BTW The OSF has been considering writing more clear guidelines for "outside" (non-Center for Open Science employees) contributions for a long time, so you are kind of my test case for how to do that and still give people realistic expectations, previously outside PRs were usually merged on a pretty arbitrary basis or just blanket refused, so we are trying to get away from that.

felliott commented 4 years ago

Hey @allsey87,

This is a great idea, but tough to implement with the current design of MFR and OSF. Adding preload=none is pretty straightforward, but supplying an arbitrary poster or pulling out a still from a timestamp will be a lot more work. In the meantime, I've added a PR to turn off preload an provide a generic static image as the poster.

Cheers, @felliott