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.

Sign up for GitHub

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

Jump to bottom

Inspect Media Size #2188

Merged
merged 2 commits into from
Jan 18, 2021
Merged

Inspect Media Size #2188

merged 2 commits into from
Jan 18, 2021

Conversation

lkiesow
Copy link
Member

@lkiesow lkiesow commented Jan 4, 2021 •
edited
Loading

This patch makes the media inspection service add the media file size to
inspected media package elements.

This fixes #1169
This fixes #1177

Your pull request should…

@lkiesow
Inspect Media Size
50f12df 
This patch makes the media inspection service add the media file size to
inspected media package elements.

This fixes opencast#1169
This fixes opencast#1177
@lkiesow lkiesow added the elan e.V. Pull requests originating from elan e.V. label Jan 4, 2021
@lkiesow
Add File Size of Image Attachments
ea61077 
This patch adds the file size to image attachments converted via
composer service. These were previously set to 0 bytes.

This fixes opencast#1177
@gregorydlogan
Copy link
Member

Does this make sense for r/9.x?

@lkiesow
Copy link
Member Author

lkiesow commented Jan 5, 2021

It's not particular important but I can quickly do that.

@lkiesow lkiesow changed the base branch from develop to r/9.x January 5, 2021 18:38
@lkiesow
Copy link
Member Author

lkiesow commented Jan 5, 2021

Turns out I've built this on r/9.x already :D

pascalseeland
pascalseeland approved these changes Jan 15, 2021
View reviewed changes
Copy link
Contributor

@pascalseeland pascalseeland left a comment

Choose a reason for hiding this comment

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

Looks fine.

@felix-itz
Copy link
Contributor

image

Backported both commits to r/8.x and appears to work as expected:

  • Tracks, segment+preview, feed+preview, search+preview images get their size set without running a dedicated inspect WO.
  • Manually attached preview images (through Admin UI -> Assets -> Add Asset -> Preview image) won't get their size set (this might be an issue with our configuration; for example bmp attachment is not converted to any web format)
  • presentation/timeline+preview and presenter/player+preview do not get their size set. They are generated by the timelinepreviews and cover-image WOH respectively.

@lkiesow
Copy link
Member Author

lkiesow commented Jan 15, 2021

This only claims to fix #1169 and #1177 which does not include the last two items. There are also several more cases where you will not get a size. This will not make this a guaranteed attribute. Are you okay with merging this like it is now, @felix-itz?

@Rillke
Copy link
Member

Rillke commented Jan 18, 2021

Please merge. The comment, @felix-itz left was just for documentary purposes.

@lkiesow
Copy link
Member Author

lkiesow commented Jan 18, 2021

Okay → merging.
@felix-itz, if you have a detailed summary of what to add, feel free to open an issue for that.

@lkiesow lkiesow merged commit c18d8ae into opencast:r/9.x Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pascalseeland pascalseeland pascalseeland approved these changes

Assignees

@pascalseeland pascalseeland

Labels
elan e.V. Pull requests originating from elan e.V.
Projects
None yet
Milestone
No milestone
5 participants
@lkiesow @gregorydlogan @felix-itz @Rillke @pascalseeland