Skip to content

Bound exp-Golomb prefix k < 30 in dec_vlc_read_kparam0() and dec_vlc_read_1bit_read() #290

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.

Sign up for GitHub

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

Merged
kpchoi merged 1 commit into from
Sep 29, 2026
Merged

Bound exp-Golomb prefix k < 30 in dec_vlc_read_kparam0() and dec_vlc_read_1bit_read() #290

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/oapv_vlc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ int oapve_vlc_get_coef_rate(oapve_core_t* core, s16* coef, int c)

static int dec_vlc_read_kparam0(oapv_bs_t *bs)
{
int symbol;
u32 symbol;

@kpchoi kpchoi Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changing symbol from int to u32 is not needed for correctness. The additions are already done in unsigned arithmetic (int + u32, int + u64), so there is no signed overflow. With the k check after the loop, the result always fits in int.

Still, the change is fine to keep as a cosmetic change. It matches dec_vlc_read(), which already uses u32 symbol, and it keeps code reviewers and coding agents from mistaking this pattern for a signed overflow.

int flag, k;

symbol = 2;
Expand All @@ -679,7 +679,7 @@ static int dec_vlc_read_kparam0(oapv_bs_t *bs)
k++;
}
}
oapv_assert_rv(k < 32, -1); /* prevent too large (impossible) k value */
oapv_assert_rv(k < 30, -1); /* prevent too large (impossible) k value */

@kpchoi kpchoi Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a correct comparison. thanks.


if(k > 0) {
symbol += ((u32)0xFFFFFFFF) >> (32 - k);
Expand All @@ -693,18 +693,18 @@ static int dec_vlc_read_kparam0(oapv_bs_t *bs)
bs->code <<= k;
bs->leftbits -= k;
}
return symbol;
return (int)symbol;
}

static int dec_vlc_read_1bit_read(oapv_bs_t *bs)
{
int symbol;
u32 symbol;
int flag, k;

if(bs->leftbits == 0) BSR_FLUSH_1BYTE(bs);
BSR_READ_1BIT(bs, flag);

symbol = (1 + flag);
symbol = (u32)(1 + flag);
k = 0;
if(flag) { // parse_exp_golomb
while(1) {
Expand All @@ -720,7 +720,7 @@ static int dec_vlc_read_1bit_read(oapv_bs_t *bs)
}
}

oapv_assert_rv(k < 32, -1); /* prevent too large (impossible) k value */
oapv_assert_rv(k < 30, -1); /* prevent too large (impossible) k value */

if(k > 0) {
symbol += ((u32)0xFFFFFFFF) >> (32 - k);
Expand All @@ -734,7 +734,7 @@ static int dec_vlc_read_1bit_read(oapv_bs_t *bs)
bs->code <<= k;
bs->leftbits -= k;
}
return symbol;
return (int)symbol;
}

static int dec_vlc_read(oapv_bs_t *bs, int k)
Expand Down
Loading