[PATCH 1/6] avcodec/g723_1enc: Remove unnecessary av_clipl_int32()
partial_corr is an int16_t and so the av_clipl_int32() never clips and can be removed. This also avoids undefined left-shifts of negative numbers. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/g723_1enc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libavcodec/g723_1enc.c b/libavcodec/g723_1enc.c index f3baf7b4ec..a22985ca44 100644 --- a/libavcodec/g723_1enc.c +++ b/libavcodec/g723_1enc.c @@ -200,8 +200,7 @@ static void levinson_durbin(int16_t *lpc, int16_t *autocorr, int16_t error) partial_corr = temp / (error << 1); - lpc[i] = av_clipl_int32((int64_t) (partial_corr << 14) + - (1 << 15)) >> 16; + lpc[i] = (partial_corr + (1 << 1)) >> 2; /* Update the prediction error */ temp = MULL2(temp, partial_corr); -- 2.34.1
Affected the acodec-g723_1 FATE-test. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/g723_1enc.c | 71 +++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/libavcodec/g723_1enc.c b/libavcodec/g723_1enc.c index a22985ca44..8466067185 100644 --- a/libavcodec/g723_1enc.c +++ b/libavcodec/g723_1enc.c @@ -126,7 +126,7 @@ static void highpass_filter(int16_t *buf, int16_t *fir, int *iir) { int i; for (i = 0; i < FRAME_LEN; i++) { - *iir = (buf[i] << 15) + ((-*fir) << 15) + MULL2(*iir, 0x7f00); + *iir = (buf[i] - *fir) * (1 << 15) + MULL2(*iir, 0x7f00); *fir = buf[i]; buf[i] = av_clipl_int32((int64_t)*iir + (1 << 15)) >> 16; } @@ -166,7 +166,7 @@ static void comp_autocorr(int16_t *buf, int16_t *autocorr) } else { for (i = 1; i <= LPC_ORDER; i++) { temp = ff_dot_product(vector, vector + i, LPC_FRAME - i); - temp = MULL2((temp << scale), binomial_window[i - 1]); + temp = MULL2(temp * (1 << scale), binomial_window[i - 1]); autocorr[i] = av_clipl_int32((int64_t) temp + (1 << 15)) >> 16; } } @@ -193,7 +193,7 @@ static void levinson_durbin(int16_t *lpc, int16_t *autocorr, int16_t error) temp = 0; for (j = 0; j < i; j++) temp -= lpc[j] * autocorr[i - j - 1]; - temp = ((autocorr[i] << 13) + temp) << 3; + temp = (autocorr[i] * (1 << 13) + temp) * (1 << 3); if (FFABS(temp) >= (error << 16)) break; @@ -209,8 +209,8 @@ static void levinson_durbin(int16_t *lpc, int16_t *autocorr, int16_t error) memcpy(vector, lpc, i * sizeof(int16_t)); for (j = 0; j < i; j++) { - temp = partial_corr * vector[i - j - 1] << 1; - lpc[j] = av_clipl_int32((int64_t) (lpc[j] << 16) - temp + + temp = partial_corr * vector[i - j - 1] * 2; + lpc[j] = av_clipl_int32((int64_t) (lpc[j] * (1 << 16)) - temp + (1 << 15)) >> 16; } } @@ -259,9 +259,9 @@ static void lpc2lsp(int16_t *lpc, int16_t *prev_lsp, int16_t *lsp) /* Compute the remaining coefficients */ for (i = 0; i < LPC_ORDER / 2; i++) { /* f1 */ - f[2 * i + 2] = -f[2 * i] - ((lsp[i] + lsp[LPC_ORDER - 1 - i]) << 12); + f[2 * i + 2] = -f[2 * i] - (lsp[i] + lsp[LPC_ORDER - 1 - i]) * (1 << 12); /* f2 */ - f[2 * i + 3] = f[2 * i + 1] - ((lsp[i] - lsp[LPC_ORDER - 1 - i]) << 12); + f[2 * i + 3] = f[2 * i + 1] - (lsp[i] - lsp[LPC_ORDER - 1 - i]) * (1 << 12); } /* Divide f1[5] and f2[5] by 2 for use in polynomial evaluation */ @@ -276,7 +276,7 @@ static void lpc2lsp(int16_t *lpc, int16_t *prev_lsp, int16_t *lsp) shift = ff_g723_1_normalize_bits(max, 31); for (i = 0; i < LPC_ORDER + 2; i++) - f[i] = av_clipl_int32((int64_t) (f[i] << shift) + (1 << 15)) >> 16; + f[i] = av_clipl_int32((int64_t) (f[i] * (1 << shift)) + (1 << 15)) >> 16; /** * Evaluate F1 and F2 at uniform intervals of pi/256 along the @@ -293,7 +293,7 @@ static void lpc2lsp(int16_t *lpc, int16_t *prev_lsp, int16_t *lsp) temp = 0; for (j = 0; j <= LPC_ORDER / 2; j++) temp += f[LPC_ORDER - 2 * j + p] * ff_g723_1_cos_tab[i * j % COS_TBL_SIZE]; - cur_val = av_clipl_int32(temp << 1); + cur_val = av_clipl_int32(temp * 2); /* Check for sign change, indicating a zero crossing */ if ((cur_val ^ prev_val) < 0) { @@ -317,7 +317,7 @@ static void lpc2lsp(int16_t *lpc, int16_t *prev_lsp, int16_t *lsp) for (j = 0; j <= LPC_ORDER / 2; j++) temp += f[LPC_ORDER - 2 * j + p] * ff_g723_1_cos_tab[i * j % COS_TBL_SIZE]; - cur_val = av_clipl_int32(temp << 1); + cur_val = av_clipl_int32(temp * 2); } prev_val = cur_val; } @@ -344,7 +344,7 @@ static void lpc2lsp(int16_t *lpc, int16_t *prev_lsp, int16_t *lsp) temp[j] = (weight[j + (offset)] * ff_g723_1_lsp_band##num[i][j] + \ (1 << 14)) >> 15; \ } \ - error = ff_g723_1_dot_product(lsp + (offset), temp, size) << 1; \ + error = ff_g723_1_dot_product(lsp + (offset), temp, size) * 2; \ error -= ff_g723_1_dot_product(ff_g723_1_lsp_band##num[i], temp, size); \ if (error > max) { \ max = error; \ @@ -419,7 +419,7 @@ static void iir_filter(int16_t *fir_coef, int16_t *iir_coef, iir_coef[n - 1] * dest[m - n]; } - dest[m] = av_clipl_int32((src[m] << 16) + (filter << 3) + + dest[m] = av_clipl_int32(src[m] * (1 << 16) + filter * (1 << 3) + (1 << 15)) >> 16; } } @@ -559,7 +559,7 @@ static void comp_harmonic_coeff(int16_t *buf, int16_t pitch_lag, HFParam *hf) exp = ff_g723_1_normalize_bits(max, 31); for (i = 0; i < 15; i++) { - energy[i] = av_clipl_int32((int64_t)(energy[i] << exp) + + energy[i] = av_clipl_int32((int64_t)(energy[i] * (1 << exp)) + (1 << 15)) >> 16; } @@ -613,8 +613,8 @@ static void harmonic_filter(HFParam *hf, const int16_t *src, int16_t *dest) int i; for (i = 0; i < SUBFRAME_LEN; i++) { - int64_t temp = hf->gain * src[i - hf->index] << 1; - dest[i] = av_clipl_int32((src[i] << 16) - temp + (1 << 15)) >> 16; + int64_t temp = hf->gain * src[i - hf->index] * 2; + dest[i] = av_clipl_int32(src[i] * (1 << 16) - temp + (1 << 15)) >> 16; } } @@ -622,8 +622,8 @@ static void harmonic_noise_sub(HFParam *hf, const int16_t *src, int16_t *dest) { int i; for (i = 0; i < SUBFRAME_LEN; i++) { - int64_t temp = hf->gain * src[i - hf->index] << 1; - dest[i] = av_clipl_int32(((dest[i] - src[i]) << 16) + temp + + int64_t temp = hf->gain * src[i - hf->index] * 2; + dest[i] = av_clipl_int32((dest[i] - src[i]) * (1 << 16) + temp + (1 << 15)) >> 16; } } @@ -655,7 +655,7 @@ static void synth_percept_filter(int16_t *qnt_lpc, int16_t *perf_lpc, for (j = 1; j <= LPC_ORDER; j++) temp -= qnt_lpc[j - 1] * bptr_16[i - j]; - buf[i] = (src[i] << 15) + (temp << 3); + buf[i] = src[i] * (1 << 15) + temp * (1 << 3); bptr_16[i] = av_clipl_int32(buf[i] + (1 << 15)) >> 16; } @@ -665,7 +665,7 @@ static void synth_percept_filter(int16_t *qnt_lpc, int16_t *perf_lpc, fir -= perf_lpc[j - 1] * bptr_16[i - j]; iir += perf_lpc[j + LPC_ORDER - 1] * dest[i - j]; } - dest[i] = av_clipl_int32(((buf[i] + (fir << 3)) << scale) + (iir << 3) + + dest[i] = av_clipl_int32((buf[i] + fir * (1 << 3)) * (1 << scale) + iir * (1 << 3) + (1 << 15)) >> 16; } memcpy(perf_fir, buf_16 + SUBFRAME_LEN, sizeof(int16_t) * LPC_ORDER); @@ -714,23 +714,22 @@ static void acb_search(G723_1_ChannelContext *p, int16_t *residual, temp = 0; for (k = 0; k <= j; k++) temp += residual[PITCH_ORDER - 1 + k] * impulse_resp[j - k]; - flt_buf[PITCH_ORDER - 1][j] = av_clipl_int32((temp << 1) + - (1 << 15)) >> 16; + flt_buf[PITCH_ORDER - 1][j] = av_clipl_int32(temp * 2 + (1 << 15)) >> 16; } for (j = PITCH_ORDER - 2; j >= 0; j--) { - flt_buf[j][0] = ((residual[j] << 13) + (1 << 14)) >> 15; + flt_buf[j][0] = (residual[j] + (1 << 1)) >> 2; for (k = 1; k < SUBFRAME_LEN; k++) { - temp = (flt_buf[j + 1][k - 1] << 15) + + temp = flt_buf[j + 1][k - 1] * (1 << 15) + residual[j] * impulse_resp[k]; - flt_buf[j][k] = av_clipl_int32((temp << 1) + (1 << 15)) >> 16; + flt_buf[j][k] = av_clipl_int32(temp * 2 + (1 << 15)) >> 16; } } /* Compute crosscorrelation with the signal */ for (j = 0; j < PITCH_ORDER; j++) { temp = ff_dot_product(buf, flt_buf[j], SUBFRAME_LEN); - ccr_buf[count++] = av_clipl_int32(temp << 1); + ccr_buf[count++] = av_clipl_int32(temp * 2); } /* Compute energies */ @@ -742,7 +741,7 @@ static void acb_search(G723_1_ChannelContext *p, int16_t *residual, for (j = 1; j < PITCH_ORDER; j++) { for (k = 0; k < j; k++) { temp = ff_dot_product(flt_buf[j], flt_buf[k], SUBFRAME_LEN); - ccr_buf[count++] = av_clipl_int32(temp << 2); + ccr_buf[count++] = av_clipl_int32(temp * (1 << 2)); } } } @@ -755,7 +754,7 @@ static void acb_search(G723_1_ChannelContext *p, int16_t *residual, temp = ff_g723_1_normalize_bits(max, 31); for (i = 0; i < 20 * iter; i++) - ccr_buf[i] = av_clipl_int32((int64_t) (ccr_buf[i] << temp) + + ccr_buf[i] = av_clipl_int32((int64_t) (ccr_buf[i] * (1 << temp)) + (1 << 15)) >> 16; max = 0; @@ -803,11 +802,11 @@ static void sub_acb_contrib(const int16_t *residual, const int16_t *impulse_resp int i, j; /* Subtract adaptive CB contribution to obtain the residual */ for (i = 0; i < SUBFRAME_LEN; i++) { - int64_t temp = buf[i] << 14; + int64_t temp = buf[i] * (1 << 14); for (j = 0; j <= i; j++) temp -= residual[j] * impulse_resp[i - j]; - buf[i] = av_clipl_int32((temp << 2) + (1 << 15)) >> 16; + buf[i] = av_clipl_int32(temp * (1 << 2) + (1 << 15)) >> 16; } } @@ -851,7 +850,7 @@ static void get_fcb_param(FCBParam *optim, int16_t *impulse_resp, for (i = 1; i < SUBFRAME_LEN; i++) { temp = ff_g723_1_dot_product(temp_corr + i, temp_corr, SUBFRAME_LEN - i); - impulse_corr[i] = av_clipl_int32((temp << scale) + (1 << 15)) >> 16; + impulse_corr[i] = av_clipl_int32(temp * (1 << scale) + (1 << 15)) >> 16; } /* Compute crosscorrelation of impulse response with residual signal */ @@ -861,7 +860,7 @@ static void get_fcb_param(FCBParam *optim, int16_t *impulse_resp, if (scale < 0) ccr1[i] = temp >> -scale; else - ccr1[i] = av_clipl_int32(temp << scale); + ccr1[i] = av_clipl_int32(temp * (1 << scale)); } /* Search loop */ @@ -910,7 +909,7 @@ static void get_fcb_param(FCBParam *optim, int16_t *impulse_resp, continue; temp = impulse_corr[FFABS(l - param.pulse_pos[k - 1])]; temp = av_clipl_int32((int64_t) temp * - param.pulse_sign[k - 1] << 1); + param.pulse_sign[k - 1] * 2); ccr2[l] -= temp; temp = FFABS(ccr2[l]); if (temp > max) { @@ -934,17 +933,17 @@ static void get_fcb_param(FCBParam *optim, int16_t *impulse_resp, temp = 0; for (l = 0; l <= k; l++) { int prod = av_clipl_int32((int64_t) temp_corr[l] * - impulse_r[k - l] << 1); + impulse_r[k - l] * 2); temp = av_clipl_int32(temp + prod); } - temp_corr[k] = temp << 2 >> 16; + temp_corr[k] = temp >> 14; } /* Compute square of error */ err = 0; for (k = 0; k < SUBFRAME_LEN; k++) { int64_t prod; - prod = av_clipl_int32((int64_t) buf[k] * temp_corr[k] << 1); + prod = av_clipl_int32((int64_t) buf[k] * temp_corr[k] * 2); err = av_clipl_int32(err - prod); prod = av_clipl_int32((int64_t) temp_corr[k] * temp_corr[k]); err = av_clipl_int32(err + prod); @@ -1204,7 +1203,7 @@ static int g723_1_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, memmove(p->prev_excitation, p->prev_excitation + SUBFRAME_LEN, sizeof(int16_t) * (PITCH_MAX - SUBFRAME_LEN)); for (j = 0; j < SUBFRAME_LEN; j++) - in[j] = av_clip_int16((in[j] << 1) + impulse_resp[j]); + in[j] = av_clip_int16(in[j] * 2 + impulse_resp[j]); memcpy(p->prev_excitation + PITCH_MAX - SUBFRAME_LEN, in, sizeof(int16_t) * SUBFRAME_LEN); -- 2.34.1
This might happen in avio_write() if size == 0 when the direct codepath is taken. It is undefined behaviour according to the spec although it happens to work in practice. Fixes the webm-webvtt-remux FATE-test under UBSan. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/aviobuf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index b20b1a611a..5b6a42d7f4 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -231,6 +231,8 @@ void ffio_fill(AVIOContext *s, int b, int64_t count) void avio_write(AVIOContext *s, const unsigned char *buf, int size) { + if (size <= 0) + return; if (s->direct && !s->update_checksum) { avio_flush(s); writeout(s, buf, size); @@ -246,7 +248,7 @@ void avio_write(AVIOContext *s, const unsigned char *buf, int size) buf += len; size -= len; - } + } while (size > 0); } void avio_flush(AVIOContext *s) -- 2.34.1
On 9/28/2022 3:58 PM, Andreas Rheinhardt wrote:
This might happen in avio_write() if size == 0 when the direct codepath is taken. It is undefined behaviour according to the spec although it happens to work in practice. Fixes the webm-webvtt-remux FATE-test under UBSan.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/aviobuf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index b20b1a611a..5b6a42d7f4 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -231,6 +231,8 @@ void ffio_fill(AVIOContext *s, int b, int64_t count)
void avio_write(AVIOContext *s, const unsigned char *buf, int size) { + if (size <= 0) + return; if (s->direct && !s->update_checksum) { avio_flush(s); writeout(s, buf, size); @@ -246,7 +248,7 @@ void avio_write(AVIOContext *s, const unsigned char *buf, int size)
buf += len; size -= len; - } + } while (size > 0);
Why are you adding this at the end of a similar while statement? Did you mean to replace the previous one with do()?
}
void avio_flush(AVIOContext *s)
James Almer:
On 9/28/2022 3:58 PM, Andreas Rheinhardt wrote:
This might happen in avio_write() if size == 0 when the direct codepath is taken. It is undefined behaviour according to the spec although it happens to work in practice. Fixes the webm-webvtt-remux FATE-test under UBSan.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/aviobuf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index b20b1a611a..5b6a42d7f4 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -231,6 +231,8 @@ void ffio_fill(AVIOContext *s, int b, int64_t count) void avio_write(AVIOContext *s, const unsigned char *buf, int size) { + if (size <= 0) + return; if (s->direct && !s->update_checksum) { avio_flush(s); writeout(s, buf, size); @@ -246,7 +248,7 @@ void avio_write(AVIOContext *s, const unsigned char *buf, int size) buf += len; size -= len; - } + } while (size > 0);
Why are you adding this at the end of a similar while statement? Did you mean to replace the previous one with do()?
Yes, exactly. Somehow I didn't. fate passed locally (apart from the tdsc). Crazy that I forgot this. Thanks for spotting. - Andreas
Affected the rv20-1239 FATE test. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/jrevdct.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/libavcodec/jrevdct.c b/libavcodec/jrevdct.c index 36160cb663..7f1863515f 100644 --- a/libavcodec/jrevdct.c +++ b/libavcodec/jrevdct.c @@ -255,7 +255,7 @@ void ff_j_rev_dct(DCTBLOCK data) if (d0) { /* Compute a 32 bit value to assign. */ int16_t dcval = (int16_t) (d0 * (1 << PASS1_BITS)); - register int v = (dcval & 0xffff) | ((dcval * (1 << 16)) & 0xffff0000); + register unsigned v = (dcval & 0xffff) | ((uint32_t)dcval << 16); AV_WN32A(&idataptr[ 0], v); AV_WN32A(&idataptr[ 4], v); @@ -988,8 +988,8 @@ void ff_j_rev_dct4(DCTBLOCK data) /* AC terms all zero */ if (d0) { /* Compute a 32 bit value to assign. */ - int16_t dcval = (int16_t) (d0 << PASS1_BITS); - register int v = (dcval & 0xffff) | ((dcval << 16) & 0xffff0000); + int16_t dcval = (int16_t) (d0 * (1 << PASS1_BITS)); + register unsigned v = (dcval & 0xffff) | ((uint32_t)dcval << 16); AV_WN32A(&idataptr[0], v); AV_WN32A(&idataptr[4], v); @@ -1008,8 +1008,8 @@ void ff_j_rev_dct4(DCTBLOCK data) tmp2 = z1 + MULTIPLY(-d6, FIX_1_847759065); tmp3 = z1 + MULTIPLY(d2, FIX_0_765366865); - tmp0 = (d0 + d4) << CONST_BITS; - tmp1 = (d0 - d4) << CONST_BITS; + tmp0 = (d0 + d4) * (1 << CONST_BITS); + tmp1 = (d0 - d4) * (1 << CONST_BITS); tmp10 = tmp0 + tmp3; tmp13 = tmp0 - tmp3; @@ -1020,8 +1020,8 @@ void ff_j_rev_dct4(DCTBLOCK data) tmp2 = MULTIPLY(-d6, FIX_1_306562965); tmp3 = MULTIPLY(d6, FIX_0_541196100); - tmp0 = (d0 + d4) << CONST_BITS; - tmp1 = (d0 - d4) << CONST_BITS; + tmp0 = (d0 + d4) * (1 << CONST_BITS); + tmp1 = (d0 - d4) * (1 << CONST_BITS); tmp10 = tmp0 + tmp3; tmp13 = tmp0 - tmp3; @@ -1034,8 +1034,8 @@ void ff_j_rev_dct4(DCTBLOCK data) tmp2 = MULTIPLY(d2, FIX_0_541196100); tmp3 = MULTIPLY(d2, FIX_1_306562965); - tmp0 = (d0 + d4) << CONST_BITS; - tmp1 = (d0 - d4) << CONST_BITS; + tmp0 = (d0 + d4) * (1 << CONST_BITS); + tmp1 = (d0 - d4) * (1 << CONST_BITS); tmp10 = tmp0 + tmp3; tmp13 = tmp0 - tmp3; @@ -1043,8 +1043,8 @@ void ff_j_rev_dct4(DCTBLOCK data) tmp12 = tmp1 - tmp2; } else { /* d0 != 0, d2 == 0, d4 != 0, d6 == 0 */ - tmp10 = tmp13 = (d0 + d4) << CONST_BITS; - tmp11 = tmp12 = (d0 - d4) << CONST_BITS; + tmp10 = tmp13 = (d0 + d4) * (1 << CONST_BITS); + tmp11 = tmp12 = (d0 - d4) * (1 << CONST_BITS); } } @@ -1086,8 +1086,8 @@ void ff_j_rev_dct4(DCTBLOCK data) tmp2 = z1 + MULTIPLY(-d6, FIX_1_847759065); tmp3 = z1 + MULTIPLY(d2, FIX_0_765366865); - tmp0 = (d0 + d4) << CONST_BITS; - tmp1 = (d0 - d4) << CONST_BITS; + tmp0 = (d0 + d4) * (1 << CONST_BITS); + tmp1 = (d0 - d4) * (1 << CONST_BITS); tmp10 = tmp0 + tmp3; tmp13 = tmp0 - tmp3; @@ -1098,8 +1098,8 @@ void ff_j_rev_dct4(DCTBLOCK data) tmp2 = MULTIPLY(-d6, FIX_1_306562965); tmp3 = MULTIPLY(d6, FIX_0_541196100); - tmp0 = (d0 + d4) << CONST_BITS; - tmp1 = (d0 - d4) << CONST_BITS; + tmp0 = (d0 + d4) * (1 << CONST_BITS); + tmp1 = (d0 - d4) * (1 << CONST_BITS); tmp10 = tmp0 + tmp3; tmp13 = tmp0 - tmp3; @@ -1112,8 +1112,8 @@ void ff_j_rev_dct4(DCTBLOCK data) tmp2 = MULTIPLY(d2, FIX_0_541196100); tmp3 = MULTIPLY(d2, FIX_1_306562965); - tmp0 = (d0 + d4) << CONST_BITS; - tmp1 = (d0 - d4) << CONST_BITS; + tmp0 = (d0 + d4) * (1 << CONST_BITS); + tmp1 = (d0 - d4) * (1 << CONST_BITS); tmp10 = tmp0 + tmp3; tmp13 = tmp0 - tmp3; @@ -1121,8 +1121,8 @@ void ff_j_rev_dct4(DCTBLOCK data) tmp12 = tmp1 - tmp2; } else { /* d0 != 0, d2 == 0, d4 != 0, d6 == 0 */ - tmp10 = tmp13 = (d0 + d4) << CONST_BITS; - tmp11 = tmp12 = (d0 - d4) << CONST_BITS; + tmp10 = tmp13 = (d0 + d4) * (1 << CONST_BITS); + tmp11 = tmp12 = (d0 - d4) * (1 << CONST_BITS); } } -- 2.34.1
On Wed, Sep 28, 2022 at 08:58:16PM +0200, Andreas Rheinhardt wrote:
Affected the rv20-1239 FATE test.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/jrevdct.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)
iam a bit surprised this was not spotted long ago but LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras
Michael Niedermayer:
On Wed, Sep 28, 2022 at 08:58:16PM +0200, Andreas Rheinhardt wrote:
Affected the rv20-1239 FATE test.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/jrevdct.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)
iam a bit surprised this was not spotted long ago but LGTM
thx
Most of these changes are in ff_j_rev_dct4 which is only used if lowres == 1; I guess the fuzzer doesn't use lowres which explains why this has not been found before. - Andreas
Fixes the rv20-1239 FATE-test. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/mpegvideo.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c index d8c7bc687d..5095149eaa 100644 --- a/libavcodec/mpegvideo.c +++ b/libavcodec/mpegvideo.c @@ -843,7 +843,7 @@ static inline int hpel_motion_lowres(MpegEncContext *s, s->vdsp.emulated_edge_mc(s->sc.edge_emu_buffer, src, s->linesize, s->linesize, w + 1, (h + 1) << field_based, - src_x, src_y << field_based, + src_x, src_y * (1 << field_based), h_edge_pos, v_edge_pos); src = s->sc.edge_emu_buffer; emu = 1; @@ -945,7 +945,7 @@ static av_always_inline void mpeg_motion_lowres(MpegEncContext *s, s->vdsp.emulated_edge_mc(s->sc.edge_emu_buffer, ptr_y, linesize >> field_based, linesize >> field_based, 17, 17 + field_based, - src_x, src_y << field_based, h_edge_pos, + src_x, src_y * (1 << field_based), h_edge_pos, v_edge_pos); ptr_y = s->sc.edge_emu_buffer; if (!CONFIG_GRAY || !(s->avctx->flags & AV_CODEC_FLAG_GRAY)) { @@ -956,12 +956,12 @@ static av_always_inline void mpeg_motion_lowres(MpegEncContext *s, s->vdsp.emulated_edge_mc(ubuf, ptr_cb, uvlinesize >> field_based, uvlinesize >> field_based, 9, 9 + field_based, - uvsrc_x, uvsrc_y << field_based, + uvsrc_x, uvsrc_y * (1 << field_based), h_edge_pos >> 1, v_edge_pos >> 1); s->vdsp.emulated_edge_mc(vbuf, ptr_cr, uvlinesize >> field_based,uvlinesize >> field_based, 9, 9 + field_based, - uvsrc_x, uvsrc_y << field_based, + uvsrc_x, uvsrc_y * (1 << field_based), h_edge_pos >> 1, v_edge_pos >> 1); ptr_cb = ubuf; ptr_cr = vbuf; -- 2.34.1
On Wed, Sep 28, 2022 at 08:58:17PM +0200, Andreas Rheinhardt wrote:
Fixes the rv20-1239 FATE-test.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/mpegvideo.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued
Affected the mpeg2-field-enc FATE-test. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/mpegvideo_dec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/mpegvideo_dec.c b/libavcodec/mpegvideo_dec.c index 5b37e79e36..6d1edc027a 100644 --- a/libavcodec/mpegvideo_dec.c +++ b/libavcodec/mpegvideo_dec.c @@ -446,8 +446,8 @@ int ff_mpv_frame_start(MpegEncContext *s, AVCodecContext *avctx) if (s->picture_structure != PICT_FRAME) { for (int i = 0; i < 4; i++) { if (s->picture_structure == PICT_BOTTOM_FIELD) { - s->current_picture.f->data[i] += - s->current_picture.f->linesize[i]; + s->current_picture.f->data[i] = FF_PTR_ADD(s->current_picture.f->data[i], + s->current_picture.f->linesize[i]); } s->current_picture.f->linesize[i] *= 2; s->last_picture.f->linesize[i] *= 2; -- 2.34.1
Andreas Rheinhardt:
partial_corr is an int16_t and so the av_clipl_int32() never clips and can be removed. This also avoids undefined left-shifts of negative numbers.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/g723_1enc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libavcodec/g723_1enc.c b/libavcodec/g723_1enc.c index f3baf7b4ec..a22985ca44 100644 --- a/libavcodec/g723_1enc.c +++ b/libavcodec/g723_1enc.c @@ -200,8 +200,7 @@ static void levinson_durbin(int16_t *lpc, int16_t *autocorr, int16_t error)
partial_corr = temp / (error << 1);
- lpc[i] = av_clipl_int32((int64_t) (partial_corr << 14) + - (1 << 15)) >> 16; + lpc[i] = (partial_corr + (1 << 1)) >> 2;
/* Update the prediction error */ temp = MULL2(temp, partial_corr);
Will apply the remaining patches of this patchset tomorrow unless there are objections. - Andreas
participants (3)
-
Andreas Rheinhardt -
James Almer -
Michael Niedermayer