Better localization of npostfix and ndirect bounds #392

New Issue

claunia · 2026-01-29T20:43:11Z

claunia commented

2026-01-29 20:43:11 +00:00

Originally created by @andrew-aladev on GitHub (Sep 10, 2021).

We can see the following line in docs:

Range is from 0 to (15 << NPOSTFIX) in steps of (1 << NPOSTFIX).

Placing such constants in docs/comments is not very good solution, because every brotli bindings implementation will store such constants locally. Brotli authors may update such constants - all bindings will be broken immediately. I think it will be better to store such constants in brotli library itself. I am proposing the following constants:

#define BROTLI_MIN_NPOSTFIX 0 // max exists, min doesn't

#define BROTLI_MIN_NDIRECT 0
#define BROTLI_NDIRECT_NPOSTFIX_MAX_BASE 0xf
#define BROTLI_NDIRECT_NPOSTFIX_STEP_BASE 1

Originally created by @andrew-aladev on GitHub (Sep 10, 2021). We can see the following line in [docs](https://github.com/google/brotli/blob/master/c/include/brotli/encode.h#L203): `Range is from 0 to (15 << NPOSTFIX) in steps of (1 << NPOSTFIX).` Placing such constants in docs/comments is not very good solution, because every brotli bindings implementation will store such constants locally. Brotli authors may update such constants - all bindings will be broken immediately. I think it will be better to store such constants in brotli library itself. I am proposing the following constants: ```c #define BROTLI_MIN_NPOSTFIX 0 // max exists, min doesn't #define BROTLI_MIN_NDIRECT 0 #define BROTLI_NDIRECT_NPOSTFIX_MAX_BASE 0xf #define BROTLI_NDIRECT_NPOSTFIX_STEP_BASE 1 ```

claunia closed this issue

2026-01-29 20:43:11 +00:00

claunia commented

2026-01-29 20:43:12 +00:00

@andrew-aladev commented on GitHub (Sep 10, 2021):

Also there is an issue: BROTLI_MAX_NPOSTFIX and BROTLI_MAX_NDIRECT are defined in c/common/constants.h only and these constants are not a part of public header.

@andrew-aladev commented on GitHub (Sep 10, 2021): Also there is an issue: `BROTLI_MAX_NPOSTFIX` and `BROTLI_MAX_NDIRECT` are defined in `c/common/constants.h` only and these constants are not a part of public header.

claunia commented

2026-01-29 20:43:12 +00:00

@andrew-aladev commented on GitHub (Sep 12, 2021):

PS See also ruby-brs commit

@andrew-aladev commented on GitHub (Sep 12, 2021): PS See also [ruby-brs commit](https://github.com/andrew-aladev/ruby-brs/commit/34b437a47596988eec1032344ea261ff84ef2eca)

claunia commented

2026-01-29 20:43:12 +00:00

@eustas commented on GitHub (Dec 15, 2021):

c/common/constants.h is mostly a reflection of specification, so that is why it is not a part of public API.
The constants you have mentioned, are impliccitly in spec, so we are fine adding them. (As a consequence, this is not something that could be easily changed, because it will become inconsistent with spec).

To make sure encoder does not do "impossible" things, encoder params go through "sanation", and incorrect combinations are ignored. (See e83c7b8e8f/c/enc/encode.c (L672))

@eustas commented on GitHub (Dec 15, 2021): `c/common/constants.h` is mostly a reflection of specification, so that is why it is not a part of public API. The constants you have mentioned, are impliccitly in spec, so we are fine adding them. (As a consequence, this is not something that could be easily changed, because it will become inconsistent with spec). To make sure encoder does not do "impossible" things, encoder params go through "sanation", and incorrect combinations are ignored. (See https://github.com/google/brotli/blob/e83c7b8e8fb8b696a1df6866bc46cbb76d7e0348/c/enc/encode.c#L672)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/brotli#392