Conversation
|
While this works for adding/displaying images, it creates some errors when generate image thumbnails is enabled. Using images from https://convertico.com/samples/avif/ for the test. When browsing the images from inside a gallery it constantly prints errors in the log. |
|
Ah, I forgot the pipe for avif. FFMpeg cant autodetect it so gotta make it. |
|
pipe might also be related to #6277 |
|
@DogmaDragon WIth my recent commit I got no errors using the same images you linked but on WaterFox. Feederbox is doing Feederbox things to try and make it even better but I think it works as is for now. |
Instead of providing a solution to this bug, have you considered abandoning windows altogther and/or installing vips instead? :'))))))) |
Soon enough Windows will be gone from my life once extended security updates gets cut off. |
|
@Gykes no errors in the logs/console, but some of the thumbnails are not generated/shown correctly. I censured images since I'm unsure about their license. |
Without logs it's hard for me to make a determination. I'll look further into it tonight and hopefully have it fixed tomorrow. |
|
@DogmaDragon Seems like I was able to get it to work with my recent commit on waterfox. Im also not getting anymore errors in the logs (They came back with some of my recent changes). I downloaded all the avif files from that website that you linked for testing. 
|
|
I'm not sure if it's worth merging and dealing with these issues if we have to pull in outside dependencies, when I strongly suspect it's something to do with how we pass the bytestream to ffmpeg or a regression upstream in avif where the codec is not being detected I appreciate the effort, please don't misunderstand but I think it might not be worth the effort to keep chasing this lead for now until we figure out where in ffmpeg it's failing |
I agree with you, so don't worry :P |
|
Looked into it more, avifs are not actually images under the hood, that's why ffmpeg identifies it as a MOOV. They are all single-frame videos which means they cannot be read from stdin. @WithoutPants not sure if this is worth moving forwards with if you can only write to tmp |
|
Follow up to /tmp comment, docker persists /tmp between contianer restarts as it writes to overlayfs like it would any other file. In order to truly handle it like a tmpfile, we would need to also add deleting the file after we're done reading from /tmp. I don't have any horse in the race but I think there is value to handlding any type of file just with a /tmp switch that can be added (maybe with some warning?) |
|
My preferred approach would be to use ffmpeg to handle avif files, rather than introducing more dependencies. This does leave handling avif files in zip files. Since as feeder mentioned above ffmpeg identifies avif files as video, I think we should just document that avif files in zip files are not currently supported. Dealing with video(-like) files in zips should be solved in a separate PR. |
|
I will probably just revert back then to my pre zip stuff. I'll get with DD to see how he wants to handle documentation. |
This reverts commit 334a315.
|
Per WPs request I went ahead and reverted my zip support changes and updated docs. My current implementation allowed me to add single file avifs, previews were generated, and no error in logs as seen above. |
|
I was able to test this again using individual images:
|
|
As near as I can tell, only commit 3c6c65e is actually necessary. I'm going to try verifying on windows to determine if there are any ffmpeg errors there, but on my linux machine with ffmpeg it generates the thumbnails ok. |
|
I believe that was the initial one that DD tested and received errors due to the pipe. If i'm not mistaken we need to do it via filepath instead of pipe for avif so that part is needed as well I believe. |
Yeah, that's what I'm trying to reproduce. |
|
Alright, I've done some testing against Using ffmpeg version 4.4.0, I receive the following error: Using ffmpeg version 6.1.1, thumbnails generated successfully. I get the same errors running on linux with ffmpeg version 8.0.1. So that's a very roundabout way of arriving at the same conclusion. Piping doesn't work. I'll take another look at the code. |
|
To add: I was testing using ffmpeg-7.0.2-essentials_build. |
pkg/image/thumbnail.go
Outdated
| } | ||
|
|
||
| // ffmpegImageThumbnailFromPath generates a thumbnail from a file path (used for AVIF which can't be piped) | ||
| func (e *ThumbnailEncoder) ffmpegImageThumbnailFromPath(inputPath string, maxSize int) ([]byte, error) { |
There was a problem hiding this comment.
Optional: ffmpegImageThumbnailPath is a bit less wordy.
AVIF support dropped in 5.1 LTS |
|
I removed a lot of the unused stuff. I also added the zip check logic into the image scan.go. If it detects that an avif file is in a zip then it throws a warn log and bails on that file. |
I think we should still do the check in |
|
10 minutes ✅ |
It looks like the change introduced here was to warn and return, which results in a corrupted file entry, since it hasn't been decorated as an image file. I'll make a fix to return an error instead. |
4 participants
From what I can tell ffmpeg does most of the heavy lifting for AVIF so this is a pretty small addition. I updated DLNA as well to be able to access this addition as well.
fixes: #1729