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

Add get_bits() definition #91

Closed
wants to merge 2 commits into from
Closed

Conversation

JeromeMartinez
Copy link
Contributor

There was no definition of this function.

Issue reported by @dwbuiten

ffv1.md Outdated
@@ -172,6 +172,10 @@ Several components of FFV1 are described in this document using pseudo-code. Not

`byte_aligned( )` is true if `remaining_bits_in_bitstream( NumBytes )` is a multiple of 8, otherwise false.

#### get_bits

`get_bits( i )` is the action to read the next `i` bits in the bitstream and to return the corresponding value. The current position is increased by `i`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: better term for current position?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess, I would use the term pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reused current position from line 169.
Should I change the term in the same PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I preferred the term current position over pointer but ¯\(ツ)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

blank vote, need another voter.

@JeromeMartinez
Copy link
Contributor Author

added a commit for changing "current position" to "pointer"

Copy link
Contributor

@retokromer retokromer left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelni
Copy link
Member

This definition seems unclear in relation to lest significant bit vs most significant bit in a byte first.

@JeromeMartinez
Copy link
Contributor Author

PR updated with addition of from most significant bit to least significant bit

@michaelni michaelni closed this in cfb65d4 Jan 4, 2018
@JeromeMartinez JeromeMartinez deleted the get_bits branch June 23, 2020 19:12
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.

None yet

5 participants