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

fix: GenerateVideoThumbnail #8825

Merged
merged 4 commits into from
Jun 14, 2022
Merged

Conversation

mei23
Copy link
Contributor

@mei23 mei23 commented Jun 13, 2022

What

fix #8814 (comment)

拡張子によってformatが確定するので .png 拡張子を付けてあげる必要がある。

Why

fix #8814 (comment)

Additional info (optional)

テストした。
直接JPEGを出力すればいいのでは?と思ったけど、アスペクト比を保持する手段がなくてあきらめた。

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Jun 13, 2022
@tamaina tamaina requested review from Johann150 and syuilo June 13, 2022 12:32
@mei23
Copy link
Contributor Author

mei23 commented Jun 13, 2022

なんかこれ tmpdir が消えない気がする。なぜかしら

@mei23
Copy link
Contributor Author

mei23 commented Jun 13, 2022

わかった

@acid-chicken
Copy link
Member

直接JPEGを出力すればいいのでは?と思ったけど、アスペクト比を保持する手段がなくてあきらめた。

.videoFilters([{
  filter: 'scale',
  options: {
    w: '498',
    h: '280',
    force_original_aspect_ratio: 'decrease',
  },
}])

だとどうかしら https://ffmpeg.org/ffmpeg-filters.html#scale-1

@Johann150
Copy link
Contributor

なんかこれ tmpdir が消えない気がする。なぜかしら

Maybe we should use the unsafeCleanup option for all temporary directories, so they get deleted even if they still contain files. Otherwise I think this might be an issue in the custom emoji importing and exporting queue processors too.

@mei23
Copy link
Contributor Author

mei23 commented Jun 13, 2022

上につけても下につけてもダメだわ

GenerateVideoThumbnail failed: Error: ffmpeg exited with code 1: Filtergraph 'scale=w=498:h=100:force_original_aspect_ratio=decrease' was specified through the -vf/-af/-filter option for output stream 0:0, which is fed from a complex filtergraph.
-vf/-af/-filter and -filter_complex cannot be used together for the same stream.

@mei23
Copy link
Contributor Author

mei23 commented Jun 13, 2022

Maybe we should use the unsafeCleanup option for all temporary directories, so they get deleted even if they still contain files. Otherwise I think this might be an issue in the custom emoji importing and exporting queue processors too.

Yes.
There seems to be a problem with those features for a long time, so it seems better to do so.

This reverts commit d54cf82.
@mei23
Copy link
Contributor Author

mei23 commented Jun 13, 2022

I make createTempDir a separate issues.

@acid-chicken
Copy link
Member

上につけても下につけてもダメだわ

あー screenshot-filter_complex 消化するから結合しないといけないか……

@acid-chicken
Copy link
Member

@mei23
Copy link
Contributor Author

mei23 commented Jun 13, 2022

まあ
JPEG直接出力はOptionalなのでとりあえず保留で
Cleanup出来ない件は #8826 で解決で
とりあえずこれでいいんじゃないかしら。

@syuilo syuilo merged commit 1d8ec10 into misskey-dev:develop Jun 14, 2022
@syuilo
Copy link
Member

syuilo commented Jun 14, 2022

🙏🏻

Jeder321 pushed a commit to Jeder321/misskey that referenced this pull request Jul 13, 2022
* fix: GenerateVideoThumbnail

* CHANGELOG

* fix cleanup

* Revert "fix cleanup"

This reverts commit d54cf82.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GenerateVideoThumbnail failed
4 participants