Repository navigation
File: optional width/height for visual files - #1183
Merged
Merged
Conversation
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two optional integer properties on
File:widthandheight, in pixels.Why
A file's pixel size is a property of the file. Any application laying out an
image needs it before the bytes arrive — otherwise it reserves a guessed box
and corrects it on decode, which moves everything already on screen.
We hit this as a concrete defect in Meshenger. Scrolling back through a chat of
photographs moved the reader by hundreds of pixels per screen; the worst single
row shifted 330 px, because the box was reserved from a per-device guess (aspect
1.2) while the picture was 0.46. The user's words were "очень много дерганий и
автопрокручиваний".
Our first fix put the dimensions into our own
Messageenvelope, and that iswhy this PR exists rather than a changelog entry. A photo editor or a video
editor reading the same data has no reason to look inside a messaging ontology
for a picture's size — it would re-derive it per device, or never find it.
Why not
MediaAssetMediaAssetalready carrieswidth/height, which suggests the ecosystem doeswant these two numbers. But it
requiredsobservationRef, binding them to anObservation. A photograph sent in a chat has no observation, and inventing oneto carry two integers would be worse than the problem it solves.
If you would rather these live only on
MediaAsset, we would need a way to useit without an observation — happy to send that PR instead.
Why it has to come from the schema
Fileis"additionalProperties": false. No application can add theseprivately without its envelopes failing strict validation in any reader that
enforces the schema. (We have been bitten by precisely that on another strict
ontology, so our
Filewrites stay inside the registered field set.)Compatibility
Purely additive. Both optional,
requiredunchanged, no existing field touched.A reader that does not know them behaves exactly as today; a writer that does
not measure omits them. Range is
minimum: 1— a zero-pixel image is not athing, and allowing 0 would let a bad writer reserve an empty box.
Follow-up, if this shape is welcome
We carry other file-describing facts in the wrong envelope for the same reason
(how the phone was held when a photo was taken, which recording a frame belongs
to and at what offset). We would rather propose those here, one concern at a
time, than keep a private dialect of a shared schema.