Posted by miopa on May 4, 2009 at 11:14pm
| Project: | FlashVideo |
| Version: | 6.x-1.5-rc2 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | attheshow |
| Status: | active |
Issue Summary
Often the thumbnail taken from the default thumbnail time specified in the settings is meaningless. It would be nice to be able to override this by providing "thumbnail time" field in the "Video Upload" fieldset of the node.
Comments
#1
You can already do this by enabling the FlashVideo CCK module (included with the primary module). You'll also see help pages that are enabled along with this module explaining how to set up the override field that you need.
#2
hm..
I installed Content and FlashVideo CCK and folowed the instructioons. I have the field added to the Video content type (see pic). I see the field in the create and update screens, it works well (value is saved). But the thumbnail is still created from the value specified in the FlashVideo settings.
(I also tried putting flashvideo_thumbtime as Label of the field, with the same result)
#3
I've read twice the docs, and tried everything on two servers, and it's not working. I'm filing this as a bug, thou I'm not sure if it is a problem in the code or the documentation is unclear.
I'm using Content Construction Kit (CCK) 6.x-2.2, Drupal 6.11
#4
I'm reclassifying this bug as critical. Any overriding field value is ignored!
After some debugging, I pinned the problem somewhere around this function in
flashvideo_cck.moduleand after a small hack the module works. I can guess that this is not the right solution and probably messes up something else, but in my configuration it seems to work well.<?php
/**
* Hook from the FlashVideo module that gets all params to overate.
*/
function flashvideo_cck_flashvideo_get_params($file, $flags, $params) {
$flags = 3; // this is the hack: $flags was 0
// We only want to do this on the pending files.
if (($flags > 0)) {
// We need to load the node to get the CCK variables.
$node = node_load($file->nid);
// Only if they don't already provide a FLASHVIDEO_USE_CUSTOM flag.
if (flashvideo_cck_get_overloaded_params($node, $params)) {
// Do we regenerate a thumbnail?
$params['create_thumb'] = (($flags & FLASHVIDEO_REGEN_THUMB) == FLASHVIDEO_REGEN_THUMB);
// Do we regenerate a video?
$params['create_video'] = (($flags & FLASHVIDEO_REGEN_VIDEO) == FLASHVIDEO_REGEN_VIDEO);
}
}
return $params;
}
?>
#5
This is not the answer... You're looking in the wrong place - the issue is down to flawed logic.
This part is for the thumbsecs override;
Check out flashvideo_get_convert_params;
// Set the thumbtime as a string represation given by "hh:mm:ss"$params['thumbtime'] = flashvideo_variable_get($node_type, 'thumbtime', '00:00:02');
So the params always contain a thumbtime set from the variable. Now look at the logic in flashvideo_convert;
// Set the String Thumbsize variable.if (isset($params['thumbsecs'])) && !isset($params['thumbtime'])) {
This if statement will *never* pass because as we've seen, thumbtime is always set. Not great for an override - it can only be a replacement... You can get it working by commenting out the second part of the query so it becomes;
if (isset($params['thumbsecs'])) { // && !isset($params['thumbtime'])) {It is a bit of a workaround though as it doesn't allow for thumbtime being an override as well? Apologies that I've not looked specifically into the thumbtime override but I'd say the issue is equally due to the thumbtime being manually set to the variable value.
Pobster
PS. Kudos to stirlyn
#6
I think you're at the wrong place. There is a comment above the condition you changed:
/*** If your conversion utility doesn't support the ffmpeg version of "00:00:00" for thumbnail time or
* "WIDTH x HEIGHT" for the thumbnail size, then you are more than welcome to unset the $params['thumbtime']
* and $params['thumbsize'] variables and then just use @thumbwidth, @thumbheight, or @thumbsecs in your markup
* command. Or, if you require a different string representation, then all you have to do is just overwrite the $params['thumbtime']
* and $params['thumbsize'] variables and then just use @thumbtime and @thumbsize in your markup.
*/
If I understand this correctly, the code in the condition should be executed only if you manually unset the $params['thumbtime'] variable. Also, the override is supposed to override the initial value, regardless if it's set from the variable or from the module settings field.
On the other hand, I don't have clear idea what have I actually done with my
$flagshack, but I've noticed an interesting thing. In the settings by content type for this module, there is an option "Delete Original Video" with the following comment:The thing is that now @thumbtime is overridden even when this option is enabled.
#7
The flags force conversion every time your cron is run... Although I guess your flags will only become set if you edit the video node, but that should happen regardless of whether you override for reconversion or not?
As the CCK fields are supposed to override, I can't imagine that they'd expect a casual flashvideo user to go into the code and insert an unset in a specific place. That comment looks to me to perhaps be left over from before the CCK sub-module was introduced?
Pobster
#8
Incidentally... The CCK module unsets the override checkboxes incorrectly, I can't remember what they're in as but I've changed mine to look like this;
$form[$field]['#default_value'][0]['value'] = 0;Obviously that's not relevant to this bug being fixed, but it's a good a point as any to point it out to the maintainers should they be reading this thread.
Pobster
#9
The same problem still exists in 6.x-1.5
This patch makes it work more correctly.
#10
Is it really working? I have my fields set like shown in attachment but it isn't catching. For example I set field_flashvideo_thumbsecs on node to value 25, and the thumb is still taking in 3rd second of the video as set in administration.
I tried it with and without patch on stable and development version. Am I doing something wrong?