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

enhance: 画像の圧縮(webpublic化)をクライアントサイドで行う #8176

Closed
wants to merge 2 commits into from

Conversation

mei23
Copy link
Contributor

@mei23 mei23 commented Jan 22, 2022

What

Resolve #8173

  • webpublicが不要な場合は作成しないで負荷を軽減
  • 画像の圧縮をクライアントサイドで行うように
    • 今はJPEGのみどこからアップロードしても対象だけど、なんかいい感じにしたい。
      • スイッチあちこちに付けてもわずらわしいから、投稿フォームからアップしたときのみとかでもいいかも。
    • orientationが付いてる画像をアップするとうまく回転してくれない問題あり、モジュールの問題ぽいけどよくわからないので… 誰か (@tamaina) いい感じになおして欲しい。

Why

パフォーマンスの向上とかいろいろ

Additional info (optional)

@mei23 mei23 marked this pull request as draft January 22, 2022 14:15
Comment on lines +196 to +197
webpublicNeeded = !!metadata.exif || !!metadata.icc || !!metadata.iptc || !!metadata.xmp || !!metadata.tifftagPhotoshop
|| !metadata.width || metadata.width > 2048 || !metadata.height || metadata.height > 2048;
Copy link
Contributor

Choose a reason for hiding this comment

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

判定を逆にしたほうがわかりやすいような気もする (変数名はsatisfyWebpublicとか?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ここ切り出してユニットテスト入れたい気もしてたから、そのうちどうにかしておくのだわ

@mei23 mei23 changed the title 画像の圧縮(webpublic化)をクライアントサイドで行う enhance: 画像の圧縮(webpublic化)をクライアントサイドで行う Jan 22, 2022
@tamaina
Copy link
Contributor

tamaina commented Jan 25, 2022

そもそも #8167 を実装しないと全部webpublicになっちゃうんだなこれ

orientationが付いてる画像をアップするとうまく回転してくれない問題

情報があまりにも少なすぎるんじゃぁ

@mei23
Copy link
Contributor Author

mei23 commented Jan 25, 2022

orientationが付いてる画像をアップするとうまく回転してくれない問題

browser-image-resizer が autoRotate: true, でちゃんとorientationを考慮してくれると言ってるけど
実際 https://github.com/recurser/exif-orientation-examples あたりのサンプルを上げると回転してしまう。

@mei23
Copy link
Contributor Author

mei23 commented Jan 25, 2022

そもそも #8167 を実装しないと全部webpublicになっちゃうんだなこれ

originalありwebpublic未生成にならないかしら?

@mei23
Copy link
Contributor Author

mei23 commented Jan 25, 2022

そもそも #8167 を実装しないと全部webpublicになっちゃうんだなこれ

今はJPEGのみどこからアップロードしても対象だけど、なんかいい感じにしたい。
のことかしら

@tamaina
Copy link
Contributor

tamaina commented Jan 25, 2022

originalありwebpublic未生成にならないかしら?

なぜかICCが添付されてくるのでwebpublicが生成されてしまう

今はJPEGのみどこからアップロードしても対象だけど、なんかいい感じにしたい。

yes

@mei23
Copy link
Contributor Author

mei23 commented Jan 25, 2022

なぜかICCが添付されてくるのでwebpublicが生成されてしまう

なんと

@mei23
Copy link
Contributor Author

mei23 commented Jan 25, 2022

なぜかICCが添付されてくるのでwebpublicが生成されてしまう

Firefoxは付けて来ないけど、Chromeは付けてきちゃうわね

@tamaina
Copy link
Contributor

tamaina commented Jan 25, 2022

Firefoxは付けて来ないけど、Chromeは付けてきちゃうわね

yes

@tamaina
Copy link
Contributor

tamaina commented Jan 25, 2022

  1. iccバッファを比較してsrgbっぽい感じならパススルー(でもこれそこまでする?感があって…)
  2. APIにsrgbであることを明示するオプションを付加(これもあんまりスマートではなさそう)

@tamaina
Copy link
Contributor

tamaina commented Jan 25, 2022

  1. ICCプロファイルのことは考えない

現代的なブラウザであれば無視されることはないし、AdobeRGBとかであっても表示がおかしくなることはない…はず。
3KBぐらいあって(ICCバージョンによるかも?)微妙なサイズだけど、そこはサーバー側の都合(=webpublicを作成しない)を優先

私的には3でいいと思う

@tamaina
Copy link
Contributor

tamaina commented Jan 25, 2022

別の問題: ブラウザで変換した場合問答無用でファイル名がuntitled.jpgになる

@rinsuki
Copy link
Contributor

rinsuki commented Jan 26, 2022

変なICCプロファイルつけたまま連合に流しちゃうとICCプロファイル消す系の実装で色がくすんで悲しくなりそう(Mastodonとかそうな気がする) でも実は今でも変わらない?

@tamaina
Copy link
Contributor

tamaina commented Jan 26, 2022

ICCプロファイル消す系の実装で色がくすんで悲しくなりそう(Mastodonとかそう

とりあえずMastodonは大丈夫そう。カラースペースを処理してカラースペースを尊重して縮小してるらしい。
https://p1.a9z.dev/notes/8vy1vpmc3y
https://fedibird.com/web/statuses/107684651887767932

でも実は今でも変わらない?

sharpはカラースペースをちゃんと処理してアウトプットをsRGBに変換するので、sharpを挟んでいる以上は大丈夫

https://p1.a9z.dev/notes/8vy1xqvwyh
https://fedibird.com/web/statuses/107684658087380052

@tamaina
Copy link
Contributor

tamaina commented Jan 26, 2022

Fedibirdを検証に使ってはいけなかったっぽい
https://fedibird.com/@noellabo/107687393672352512

@tamaina
Copy link
Contributor

tamaina commented Jan 26, 2022

mstdn.jpだと壊れた
https://mstdn.jp/web/statuses/107684652901008660

@tamaina
Copy link
Contributor

tamaina commented Jan 28, 2022

(ICCの有無を無視した場合に発生する)色味の問題は「サードパーティクライアントが」「変則的なカラースペースの画像をそのまま送信する」という限定的な事象で発生する上に「ICCを無視しちゃうようなクソ実装が悪い」ので気にしなくてもいいんじゃないかなと思った

@tamaina
Copy link
Contributor

tamaina commented Jan 28, 2022

ところで、iPhone SEで下の画像が送信できなかったのだけれど再現するでしょうか
(iOS Safariのインスペクトってどうやればいいんだっけか…)

https://1drv.ms/u/s!AslpdQzTI0zLjY0O4zQ0Aty6zAM25A?e=Q9KfZc

@tamaina
Copy link
Contributor

tamaina commented Jan 29, 2022

なんか直った (iOS 15.3にしたからか?)

};
resizedImage = await readAndCompressImage(file, config)
}

uploads.value.push(ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

これはif (file.type === 'image/jpeg')の前に移した方がよさそう

packages/client/src/os.ts Outdated Show resolved Hide resolved
@tamaina
Copy link
Contributor

tamaina commented Jan 29, 2022

Orientationはexifreaderまわりが悪いのね(issueあるじゃん)

ericnograles/browser-image-resizer#34

@mei23
Copy link
Contributor Author

mei23 commented Jan 29, 2022

exifreader

はちゃんと判定していてcanvasで回転しているあたりがおかしい気がしてるのだわ

@tamaina
Copy link
Contributor

tamaina commented Jan 29, 2022

browser-image-resizerでバンドルが90KiBぐらい増える…

#8216でlazy loadにしてみた

@mei23
Copy link
Contributor Author

mei23 commented Feb 1, 2022

#8216 で続いてるからこれはもういいわよね

@mei23 mei23 closed this Feb 1, 2022
@mei23 mei23 deleted the v12-8173 branch February 1, 2022 03:17
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.

画像の圧縮(webpublic化)をクライアントサイドで行う
3 participants