[FFmpeg-devel] [PATCH v5 3/3] avcodec/adpcm: Fixes output from Westwood ADPCM.

[Aidan R]

From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> on behalf of Zane van Iperen <zane at zanevaniperen.com>
Sent: 26 April 2021 01:46
To: ffmpeg-devel at ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v5 3/3] avcodec/adpcm: Fixes output from Westwood ADPCM.



On 26/4/21 6:00 am, Aidan Richmond wrote:

Patch 1/3: lgtm

Patch 2/3:
> +static int wsaud_write_init(AVFormatContext *ctx)
> +{
> +    AVStream     *st = ctx->streams[0];
> +    AVIOContext  *pb = ctx->pb;
> +
> +    /* Stream must be seekable to correctly write the file. */
> +    if (!(pb->seekable & AVIO_SEEKABLE_NORMAL)) {
> +        av_log(ctx->streams[0], AV_LOG_ERROR, "Cannot write Westwood aud to"
> +               " none seekable stream.\n");
> +        return AVERROR(EINVAL);
> +    }
> +
Typo, Non-seekable? And capitalise aud -> AUD to be consistent.

Patch 3/3:
> Fixes bug #9198
>
> Signed-off-by: Aidan Richmond <aidan.is at hotmail.co.uk>
> ---
>   libavcodec/adpcm.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
> index be14607eac..5ec9691001 100644
> --- a/libavcodec/adpcm.c
> +++ b/libavcodec/adpcm.c
> @@ -1400,16 +1400,16 @@ static int adpcm_decode_frame(AVCodecContext *avctx, void *data,
>
>                   for (n = nb_samples / 2; n > 0; n--) {
>                       int v = bytestream2_get_byteu(&gb);
> -                    *smp++ = adpcm_ima_expand_nibble(&c->status[channel], v >> 4  , 3);
>                       *smp++ = adpcm_ima_expand_nibble(&c->status[channel], v & 0x0F, 3);
> +                    *smp++ = adpcm_ima_expand_nibble(&c->status[channel], v >> 4  , 3);
>                   }
>               }
>           } else {
>               for (n = nb_samples / 2; n > 0; n--) {
>                   for (channel = 0; channel < avctx->channels; channel++) {
>                       int v = bytestream2_get_byteu(&gb);
> -                    *samples++  = adpcm_ima_expand_nibble(&c->status[channel], v >> 4  , 3);
> -                    samples[st] = adpcm_ima_expand_nibble(&c->status[channel], v & 0x0F, 3);
> +                    *samples++  = adpcm_ima_expand_nibble(&c->status[channel], v & 0x0F, 3);
> +                    samples[st] = adpcm_ima_expand_nibble(&c->status[channel], v >> 4  , 3);
>                   }
>                   samples += avctx->channels;
>               }
>

As Andreas said in the previous patch, you need to update the FATE test.

If it's easier (and with your permission), I can just do this and squash it into the commit
before I apply it. Might as well fix the typo in 2/3 too.

See https://github.com/vs49688/FFmpeg/commit/806f982ca2c310d6d639a3e554735b41b67c9259.patch


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel at ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

If you don't mind doing the last bit to get it to a mergable state, I'm happy to give my permission.

Thanks for all the feedback and the back and forth to work this into something mergeable, I really appreciate the patience and support.