Skip to content
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

Thumbnail server: Some fixes #3087

Merged
merged 6 commits into from
Nov 23, 2019
Merged

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Nov 20, 2019

  • Use send_response_only() to avoid logging (at ERROR level) successful thumbnail loads
  • Name regular expression groups, access matches by name
  • Use match() instead of findall(), to avoid unnecessary nesting of result
  • Look up File object from supplied ID earlier, return 404 on lookup failures
    (This covers both the previous "undefined" condition, and IDs that aren't recognized)
  • Fix output path for thumbnail generation (don't override with legacy/compatibility path)
  • Add overrides for logging functions:
    • log to Python logging instance
    • don't log server address (always 127.0.0.1), timestamp

@ferdnyc ferdnyc requested a review from jonoomph November 20, 2019 22:14
Comment on lines +154 to +163
if not os.path.exists(thumb_path):
if file_frame == 1:
# Try ID with no frame # (for backwards compatibility)
alt_path = os.path.join(info.THUMBNAIL_PATH, "%s.png" % file_id)
else:
# Try with ID and frame # in filename (for backwards compatibility)
alt_path = os.path.join(info.THUMBNAIL_PATH, "%s-%s.png" % (file_id, file_frame))

if os.path.exists(alt_path):
thumb_path = alt_path
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonoomph

This relates to this item: "Fix output path for thumbnail generation (don't override with legacy/compatibility path)"

I noticed that, because thumb_path was being overridden by the legacy locations before it was passed to GenerateThumbnail(), the thumbnails were always being generated at the "backwards compatibility" path. That's now fixed, and thumbnails should be created as ProjectName_assets/thumbnail/FILE_ID/frame#.png as presumably intended.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 23, 2019

This fixes the issue, at least for me, of an otherwise non-functional thumbnail server on Win7 (resulting in an OpenShot that can't import any media files). So, merging...

@ferdnyc ferdnyc merged commit 2ad1854 into OpenShot:develop Nov 23, 2019
@ferdnyc ferdnyc deleted the thumb-server-nolog branch November 28, 2019 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant