This repository was archived by the owner on Dec 17, 2020. It is now read-only.
Add support methods for AVIF changes to parser #6
Conversation
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
Also, check for overflowing the capacity on try_reserve and add tests.
Previously, the argument (named `new_cap`) was unconditionally added to the existing capacity and the storage was reallocated. This seems to be inconsistent with existing usage (and the Vec::reserve interface), which call reserve with the number of additional elements which are about to be added to the collection, presumably to avoid multiple allocations. This changes the name of the argument to `additional` and changes the logic to allocate space for that many additional insertions, considering the current capacity, and not allocating unnecessarily. Also, add some additional tests and remove the superfluous `_test` suffix since test functions are already tagged with `#[test]` attributes.
kinetiknz
approved these changes
Feb 27, 2020
Collaborator
kinetiknz
left a comment
kinetiknz
left a comment
There was a problem hiding this comment.
Nice find in try_reserve! This doesn't change behaviour since we always passed an empty Vec from mp4parse-rust, but it's good to fix anyway.
Other changes look good to merge, but I'd prefer to implement try_read_to_end inside mp4parse-rust, since it's just building on try_reserve from this crate.
Contributor
Author
|
I'll implement your feedback and upload a new revision. Though I'm going to go through the rest of your feedback on mozilla/mp4parse-rust#193 first in case I need to make any additional changes to this crate. |
Also, add try_reserve_idempotent test to check against the specific regression in try_reserve.
Contributor
Author
|
Changes on this side are done; ready for re-review |
Collaborator
|
Thanks! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See mozilla/mp4parse-rust#193