[FFmpeg-devel] [PATCH] fix clicks in ADPCM IMA AMV decoder
Vladimir Voroshilov
voroshil
Thu Oct 4 13:56:18 CEST 2007
2007/10/4, M?ns Rullg?rd <mans at mansr.com>:
> Vitor Sessak <vitor1001 at gmail.com> writes:
>
> > Vladimir Voroshilov wrote:
> >> 2007/10/4, Vitor Sessak <vitor1001 at gmail.com>:
> >>> Hi
> >>>
> >>> Vladimir Voroshilov wrote:
> >>>>>>>>> Vladimir Voroshilov wrote:
> >>>>>>>>>> Hi, All
> >>>>>>>>>>
> >>>>>> Index: libavcodec/adpcm.c
> >>>>>> ===================================================================
> >>>>>> --- libavcodec/adpcm.c (revision 10652)
> >>>>>> +++ libavcodec/adpcm.c (working copy)
> >>>>>> @@ -1184,10 +1184,8 @@
> >>>>>> break;
> >>>>>> case CODEC_ID_ADPCM_IMA_AMV:
> >>>>>> case CODEC_ID_ADPCM_IMA_SMJPEG:
> >>>>>> - c->status[0].predictor = *src;
> >>>>>> - src += 2;
> >>>>>> - c->status[0].step_index = *src++;
> >>>>>> - src++; /* skip another byte before getting to the meat */
> >>>>>> + c->status[0].predictor = (signed short)bytestream_get_le16(&src);
> >>>>>> + c->status[0].step_index = bytestream_get_le16(&src);
> >>>>> I suppose the signed short cast is useless. If so, please remove it.
> >>>>> If you remove it, you could also align the = sign.
> >>>> Signed cast is required (exactly this signed cast removes clicks in sound).
> >>> Would just this do it?
> >>>
> >>> c->status[0].predictor = (signed) bytestream_get_le16(&src);
> >>
> >> No, "(signed)" cast does not work.
> >
> > Ok... I promise I'll test it before suggesting next time :-)
>
> The cast makes a difference here because bytestream_get_le16() returns
> an unsigned int with the high 16 bits zero. The cast to signed short
> and the following implicit widening cast act to sign-extend the low 16
> bits to 32 bits when storing the value in an int.
>
> Casting to signed short is actually not entirely correct. Since the
> size of the value to be sign-extended is exactly 16 bits, a cast to
> int16_t should be used. We cannot know for sure that short is in fact
> 16 bits, even if this virtually always is the case.
Code around uses int16_t too.
So i'll commit this trivial fix in few hours if Michael will not say against.
--
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
More information about the ffmpeg-devel
mailing list