WIP: Adds mediainfo sniffing and auto-thumbnailing with ffmpeg.#23
WIP: Adds mediainfo sniffing and auto-thumbnailing with ffmpeg.#23tomatohater wants to merge 4 commits intotorchbox:mainfrom
Conversation
nimasmi
left a comment
There was a problem hiding this comment.
Thanks, @tomatohater, this initially looks good.
I haven't gone so far as testing it, though I think that some exception handling should be introduced (in media_sniff() perhaps?) in case the return from ffmpeg isn't as expected.
|
|
||
| # Try to scrape a thumbnail from video | ||
| if hasattr(settings, 'WAGTAILMEDIA_FFMPEG_CMD')\ | ||
| and not instance.thumbnail: |
There was a problem hiding this comment.
NB this leaves no way to leave a thumbnail blank. I'm happy with this, but just pointing it out for discussion if anyone has an opinion.
There was a problem hiding this comment.
I agree with you here. I think I'll add a "No thumbnail" checkbox to the Media admin UI.
| data = sniff_media_data(instance.file.path) | ||
| if data: | ||
| duration = int(float(data['format']['duration'])) | ||
| Media.objects.filter(pk=instance.pk).update(duration=duration, mediainfo=data) |
There was a problem hiding this comment.
I agree with your comment that just dumping this data dictionary into a text field feels wasteful.
There was a problem hiding this comment.
I felt compelled to store the results of the media info extraction alongside the Media object. Maybe it should be called "raw_mediainfo" or something. I'm not sure how wasteful it is. Probably less wasteful than re-extracting via ffmpeg each time.
I don't love that it's a Textfield.... I'd prefer a JSONField but wanted to maximize compatibility.
|
Thanks for your comments, @nimasmi. I wanted to get feedback on the direction before going too far. I'll add the tests and exception handling to this PR (as well as the other tweaks already mentioned). |
|
Marking this PR as draft since it’s missing a lot to be considered for merging. I see @thenewguy closed #5 on the basis that this was outside of the scope of this project (#5 (comment)) – won’t close this PR for now but we should discuss this before anyone spends more time on this. |
ddccf3a to
b39f564
Compare
Don't merge this. Tests are not written yet and error handling needs to be improved, but I'd like to get some eyes on this before going much further. Thoughts?
Per issue #5 , this merge adds support for metadata extraction (duration, height, width) and thumbnailing if ffmpeg is installed and a path specified in settings.
A few notes: