diff mbox series

[FFmpeg-devel] avcodec/adpcm: take into account block_align when decoding ADPCM_PSX

Message ID 20200911112939.13950-1-onemda@gmail.com
State Accepted
Commit ca49476ace90ddebc5f92d9d82297f77e528c21e
Headers show
Series [FFmpeg-devel] avcodec/adpcm: take into account block_align when decoding ADPCM_PSX | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Paul B Mahol Sept. 11, 2020, 11:29 a.m. UTC
Should reduce decoding overhead.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavcodec/adpcm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Zane van Iperen Sept. 11, 2020, 1:14 p.m. UTC | #1
On Fri, 11 Sep 2020 13:29:39 +0200
"Paul B Mahol" <onemda@gmail.com> wrote:

> 
> Should reduce decoding overhead.
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavcodec/adpcm.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
> index 71e37efde7..e409a3aa6a 100644
> --- a/libavcodec/adpcm.c
> +++ b/libavcodec/adpcm.c
> @@ -1966,11 +1966,13 @@ static int adpcm_decode_frame(AVCodecContext *avctx, void *data,
>          }
>          break;
>      case AV_CODEC_ID_ADPCM_PSX:
> +        for (int block = 0; block < avpkt->size / FFMAX(avctx->block_align, 16 * avctx->channels); block++) {
> +            int nb_samples_per_block = 28 * FFMAX(avctx->block_align, 16 * avctx->channels) / (16 * avctx->channels);

Shouldn't get_nb_samples() be updated instead?
Paul B Mahol Sept. 11, 2020, 1:27 p.m. UTC | #2
On Fri, Sep 11, 2020 at 01:14:02PM +0000, Zane van Iperen wrote:
> On Fri, 11 Sep 2020 13:29:39 +0200
> "Paul B Mahol" <onemda@gmail.com> wrote:
> 
> > 
> > Should reduce decoding overhead.
> > 
> > Signed-off-by: Paul B Mahol <onemda@gmail.com>
> > ---
> >  libavcodec/adpcm.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
> > index 71e37efde7..e409a3aa6a 100644
> > --- a/libavcodec/adpcm.c
> > +++ b/libavcodec/adpcm.c
> > @@ -1966,11 +1966,13 @@ static int adpcm_decode_frame(AVCodecContext *avctx, void *data,
> >          }
> >          break;
> >      case AV_CODEC_ID_ADPCM_PSX:
> > +        for (int block = 0; block < avpkt->size / FFMAX(avctx->block_align, 16 * avctx->channels); block++) {
> > +            int nb_samples_per_block = 28 * FFMAX(avctx->block_align, 16 * avctx->channels) / (16 * avctx->channels);
> 
> Shouldn't get_nb_samples() be updated instead?

Nope.

get_nb_samples() gives number of samples in single packet depending on its size.
We here uses blocks and block_align to decode mutliple frames at once.

Imagine decoding stereo int16 pcm so that you pick only 4 bytes each time.
This would give so big overhead that it would be not usable at all.
Some ADPCM codecs just do this, and thus in player give excessive CPU usage.
Also when decoding with ffmpeg -i input -f null -, realtime speed is nowhere
near optimum value. In my case with this patch and modified demuxer to return
mutliple frames it changes from 50x to >1000x speed.
Zane van Iperen Sept. 12, 2020, 12:45 p.m. UTC | #3
On Fri, 11 Sep 2020 15:27:42 +0200
"Paul B Mahol" <onemda@gmail.com> wrote:

> get_nb_samples() gives number of samples in single packet depending on its size.
> We here uses blocks and block_align to decode mutliple frames at once.
> 
> Imagine decoding stereo int16 pcm so that you pick only 4 bytes each time.
> This would give so big overhead that it would be not usable at all.
> Some ADPCM codecs just do this, and thus in player give excessive CPU usage.
> Also when decoding with ffmpeg -i input -f null -, realtime speed is nowhere
> near optimum value. In my case with this patch and modified demuxer to return
> mutliple frames it changes from 50x to >1000x speed.

I just applied the same changes to the argo decoder and demuxer and saw a
similar speedup. It's insane how much of a difference it makes. (I'll 
send a patch for it after I've dealt with the BRP demuxer).

Also, this LGTM.
diff mbox series

Patch

diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
index 71e37efde7..e409a3aa6a 100644
--- a/libavcodec/adpcm.c
+++ b/libavcodec/adpcm.c
@@ -1966,11 +1966,13 @@  static int adpcm_decode_frame(AVCodecContext *avctx, void *data,
         }
         break;
     case AV_CODEC_ID_ADPCM_PSX:
+        for (int block = 0; block < avpkt->size / FFMAX(avctx->block_align, 16 * avctx->channels); block++) {
+            int nb_samples_per_block = 28 * FFMAX(avctx->block_align, 16 * avctx->channels) / (16 * avctx->channels);
         for (channel = 0; channel < avctx->channels; channel++) {
-            samples = samples_p[channel];
+            samples = samples_p[channel] + block * nb_samples_per_block;
 
             /* Read in every sample for this channel.  */
-            for (i = 0; i < nb_samples / 28; i++) {
+            for (i = 0; i < nb_samples_per_block / 28; i++) {
                 int filter, shift, flag, byte;
 
                 filter = bytestream2_get_byteu(&gb);
@@ -2001,6 +2003,7 @@  static int adpcm_decode_frame(AVCodecContext *avctx, void *data,
                 }
             }
         }
+        }
         break;
     case AV_CODEC_ID_ADPCM_ARGO:
         /*