-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include video durations? #74
Comments
+1 for retrieving video duration |
+1 |
@ttempleton Would you be interested in a PR for this? I could definitely add support for Vimeo and Youtube. Vimeo returns duration in the oembed response so it would be easy to grab. Youtube's duration is deep within the |
@johndwells We only really support YouTube and Vimeo for the other video-specific embedded asset methods, so it'd be fine if video durations only supported those too. But if you'd like to work on a PR for adding video durations, then we'd definitely merge that. |
I'm reconsidering submitting a PR for this, and instead submitting a
separate PR to add in some event hooks so that things like this (and
others) can be done instead.
My main thinking is that the underlying library, Embed, does not provide an
interface for accessing vimeo duration, and I think that you'd probably
want to stay within the lines of what Embed provides (doing otherwise may
prove a slippery slope!). But also, I think that Embedded Assets could
benefit more, sooner, from having some simple event hooks in place that let
others extend as necessary for such unique circumstances as this.
- J
…On Mon, Oct 19, 2020 at 2:02 AM Thomas Templeton ***@***.***> wrote:
@johndwells <https://github.com/johndwells> We only really support
YouTube and Vimeo for the other video-specific embedded asset methods, so
it'd be fine if video durations only supported those too. But if you'd like
to work on a PR for adding video durations, then we'd definitely merge that.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#74 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABAXXSQAVPGYFDWJDUZRVDSLPP7XANCNFSM4FJIFPHA>
.
--
▂
*ONE DARNLEY ROAD*
*John D Wells*
Technical Director
1C Darnley Road, London E9 6QH
+44 (0) 208 5259 292
http://onedarnleyroad.com
http://facebook.com/onedarnleyroad
http://twitter.com/onedarnleyroad
|
I don't necessarily have a problem with adding functionality that isn't directly provided by Embed. Having thought about it a bit more, though, I do agree with the slippery slope comment in that we shouldn't keep adding embedded asset functionality that's only relevant for certain types of embedded assets or for certain providers. If there were event hooks to allow extending to include video durations or other video-specific information, that seems like a good solution to that situation. |
@ttempleton I've had a play with creating an event which allows me to append arbitrary additional information (in this case, duration) to the array returned by the adapter. But then encountered 2 hurdles:
This means that even if #1 is addressed to quietly discard/skip any non-supported properties, the custom array values are entirely lost once the EmbeddedAsset is serialized and stored. I can think of one way to work around this, but it would be significant: allow plugins to provide their own EmbeddedAsset model. This would entail making an EmbeddedAssetInterface, allowing plugins to register their own, etc... quite convoluted, possibly riddled with pitfalls, though it would be the most flexible down the road. A simpler solution might be to introduce an "extras" attribute on the EmbeddedAsset model and allow that to hold arbitrary information that may be added by 3rd party plugins. Interested to hear your thoughts. To be honest, I may end up having to bail on this and resort fo my own oembed fetches, since I'm doing far more with the duration than merely displaying it. So even getting Embedded Assets to store the duration only solves one part of my puzzle. So what I suppose I'm saying is, if you feel like this ticket is just generally going down a road you don't want to travel, that's OK. I'll find another route to solving my particular issues. Thanks! |
@johndwells I think the I do like the idea of providing an interface, and in addition to allowing plugins to provide their own models for their own requirements, I could see us then providing an Embedded Video model which could have the specialist functionality like video ID/duration/etc. I definitely want to work on the |
I'm a huge fan of your
neo
plugin, and just discovered this one, which looks just like something we were needing for youtube videos.How hard would it be to add a
duration
value for the cached video data?I'm assuming (but haven't tested) that it would be possible to update cache info by doing a cache update, but doing so for an individual entry would be pretty cool.
One extra wrinkle: our site is still on Craft 2 (we use neo heavily), so I know we're not on the lastest version of the plugin.
The text was updated successfully, but these errors were encountered: