| Message ID | 20200201065954.27106-1-zane@zanevaniperen.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/adpcm_argo: simplify and move duplicated logic into a function | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Sat, Feb 01, 2020 at 06:59:59AM +0000, Zane van Iperen wrote: > Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> > --- > libavcodec/adpcm.c | 40 ++++++++++++++++++---------------------- > 1 file changed, 18 insertions(+), 22 deletions(-) it seems theres no fate test for adpcm_argo this patch looks ok but the codepath seems untested, maybe you can add a test ? thx [...]
2/2/20 4:04 am, Michael Niedermayer пишет: > > On Sat, Feb 01, 2020 at 06:59:59AM +0000, Zane van Iperen wrote: >> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> >> --- >> libavcodec/adpcm.c | 40 ++++++++++++++++++---------------------- >> 1 file changed, 18 insertions(+), 22 deletions(-) > > it seems theres no fate test for adpcm_argo > this patch looks ok but the codepath seems untested, maybe you can add a test ? > I can. Would it best be send as a v2, or as a separate patch completely? In the meantime, this patch doesn't break anything, as it passes my usual checks. These are the files that gave the most grief when writing the decoder and I know their expected hashes. $ ./ffmpeg -loglevel error -i OUTRO.ASF -map 0:a -f md5 - MD5=e0428a6c55382f48502f04b3a7f7b94b $ ./ffmpeg -loglevel error -i CBK2.asf -map 0:a -f md5 - MD5=589f3f3c518e5d58aa404b7576db5b70 > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I am the wisest man alive, for I know one thing, and that is that I know > nothing. -- Socrates > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > Zane
On Sat, Feb 01, 2020 at 11:27:47PM +0000, Zane van Iperen wrote: > 2/2/20 4:04 am, Michael Niedermayer пишет: > > > > On Sat, Feb 01, 2020 at 06:59:59AM +0000, Zane van Iperen wrote: > >> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> > >> --- > >> libavcodec/adpcm.c | 40 ++++++++++++++++++---------------------- > >> 1 file changed, 18 insertions(+), 22 deletions(-) > > > > it seems theres no fate test for adpcm_argo > > this patch looks ok but the codepath seems untested, maybe you can add a test ? > > > > I can. Would it best be send as a v2, or as a separate patch completely? whatever you prefer thx [...]
On Sat, Feb 01, 2020 at 11:27:47PM +0000, Zane van Iperen wrote: > 2/2/20 4:04 am, Michael Niedermayer пишет: > > > > On Sat, Feb 01, 2020 at 06:59:59AM +0000, Zane van Iperen wrote: > >> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> > >> --- > >> libavcodec/adpcm.c | 40 ++++++++++++++++++---------------------- > >> 1 file changed, 18 insertions(+), 22 deletions(-) > > > > it seems theres no fate test for adpcm_argo > > this patch looks ok but the codepath seems untested, maybe you can add a test ? > > > > I can. Would it best be send as a v2, or as a separate patch completely? > > In the meantime, this patch doesn't break anything, as it passes my usual checks. > These are the files that gave the most grief when writing the decoder and I > know their expected hashes. > > $ ./ffmpeg -loglevel error -i OUTRO.ASF -map 0:a -f md5 - > MD5=e0428a6c55382f48502f04b3a7f7b94b > > $ ./ffmpeg -loglevel error -i CBK2.asf -map 0:a -f md5 - > MD5=589f3f3c518e5d58aa404b7576db5b70 will apply thx [...]
diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c index dad3da28d3..9a42353351 100644 --- a/libavcodec/adpcm.c +++ b/libavcodec/adpcm.c @@ -552,9 +552,21 @@ static void adpcm_swf_decode(AVCodecContext *avctx, const uint8_t *buf, int buf_ } } -static inline int16_t adpcm_argo_expand_nibble(int nibble, int shift, int16_t prev0, int16_t prev1) +static inline int16_t adpcm_argo_expand_nibble(ADPCMChannelStatus *cs, int nibble, int control, int shift) { - return ((8 * prev0) - (4 * prev1) + (nibble * (1 << shift))) >> 2; + int sample = nibble * (1 << shift); + + if (control & 0x04) + sample += (8 * cs->sample1) - (4 * cs->sample2); + else + sample += 4 * cs->sample1; + + sample = av_clip_int16(sample >> 2); + + cs->sample2 = cs->sample1; + cs->sample1 = sample; + + return sample; } /** @@ -1805,7 +1817,7 @@ static int adpcm_decode_frame(AVCodecContext *avctx, void *data, * They should be 0 initially. */ for (channel = 0; channel < avctx->channels; channel++) { - int control, shift, sample, nibble; + int control, shift; samples = samples_p[channel]; cs = c->status + channel; @@ -1815,25 +1827,9 @@ static int adpcm_decode_frame(AVCodecContext *avctx, void *data, shift = (control >> 4) + 2; for (n = 0; n < nb_samples / 2; n++) { - sample = bytestream2_get_byteu(&gb); - - nibble = sign_extend(sample >> 4, 4); - if (control & 0x04) - *samples = adpcm_argo_expand_nibble(nibble, shift, cs->sample1, cs->sample2); - else - *samples = adpcm_argo_expand_nibble(nibble, shift, cs->sample1, cs->sample1); - - cs->sample2 = cs->sample1; - cs->sample1 = *samples++; - - nibble = sign_extend(sample >> 0, 4); - if (control & 0x04) - *samples = adpcm_argo_expand_nibble(nibble, shift, cs->sample1, cs->sample2); - else - *samples = adpcm_argo_expand_nibble(nibble, shift, cs->sample1, cs->sample1); - - cs->sample2 = cs->sample1; - cs->sample1 = *samples++; + int sample = bytestream2_get_byteu(&gb); + *samples++ = adpcm_argo_expand_nibble(cs, sign_extend(sample >> 4, 4), control, shift); + *samples++ = adpcm_argo_expand_nibble(cs, sign_extend(sample >> 0, 4), control, shift); } } break;
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> --- libavcodec/adpcm.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-)