-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
StripMetaModifier::class
#65
Conversation
@olivervogel I'm not sure this is the best way to remove |
Does the |
I believe it does strip all metadata ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should ditch the modifier here and use the output options strip
and keep
for writeToBuffer()
.
$strip = $this->strip || (is_null($this->strip) && $this->driver()->config()->strip);
$result = $image->core()->native()->writeToBuffer('.avif', [
'Q' => $this->quality,
'strip' => $strip,
'keep' => Vips\ForeignKeep::ICC,
]);
Not sure if this works in combination though. Maybe keep
means that every meta data is removed except the given value(s).
@olivervogel I think we can keep both, the modifier and the I just pushed the changes for the Jpeg encoder, let me know what you think |
src/Encoders/JpegEncoder.php
Outdated
@@ -32,6 +32,7 @@ public function encode(ImageInterface $image): EncodedImage | |||
$result = $vipsImage->writeToBuffer('.jpg', [ | |||
'Q' => $this->quality, | |||
'interlace' => $this->progressive, | |||
'strip' => $this->strip || (is_null($this->strip) && $this->driver()->config()->strip), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would remove icc as well. Shouldn't the profile be retained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its better to keep the ICC profiles as well, I'll make the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice.
Going to keep it as draft, I'm going to update the encoders next