Re: [FFmpeg-devel] [PATCH] adpcm fixes and improvements

[Paul B Mahol <onemda@gmail.com>]

On Thu, Aug 24, 2023 at 7:39 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Thu, Aug 24, 2023 at 06:26:36PM +0200, Andreas Rheinhardt wrote:
> > Michael Niedermayer:
> > > On Thu, Aug 24, 2023 at 12:51:28AM +0200, Paul B Mahol wrote:
> > >> On Thu, Aug 24, 2023 at 12:36 AM Michael Niedermayer <
> michael@niedermayer.cc>
> > >> wrote:
> > >>
> > >>> On Wed, Aug 23, 2023 at 09:04:35PM +0200, Paul B Mahol wrote:
> > >>>> On Wed, Aug 23, 2023 at 9:02 PM Michael Niedermayer <
> > >>> michael@niedermayer.cc>
> > >>>> wrote:
> > >>>>
> > >>>>> On Wed, Aug 16, 2023 at 06:53:42PM +0200, Paul B Mahol wrote:
> > >>>>>> On Wed, Aug 16, 2023 at 6:38 PM Michael Niedermayer <
> > >>>>> michael@niedermayer.cc>
> > >>>>>> wrote:
> > >>>>>>
> > >>>>>>> On Tue, Aug 15, 2023 at 04:49:05PM +0200, Paul B Mahol wrote:
> > >>>>>>>> Attached
> > >>>>>>>
> > >>>>>>> [...]
> > >>>>>>>>  adpcm.c |    2 +-
> > >>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>>>>> 3305dbe07ca935958fa213f5cadc339ad3cc3592
> > >>>>>>> 0003-avcodec-adpcm-use-already-existing-pointer-for-4xm-d.patch
> > >>>>>>>> From c6ad6dc7b8725d897e36399e5c7b8174caeb92e6 Mon Sep 17
> 00:00:00
> > >>>>> 2001
> > >>>>>>>> From: Paul B Mahol <onemda@gmail.com>
> > >>>>>>>> Date: Tue, 15 Aug 2023 14:18:47 +0200
> > >>>>>>>> Subject: [PATCH 3/4] avcodec/adpcm: use already existing pointer
> > >>> for
> > >>>>> 4xm
> > >>>>>>>>  decoder
> > >>>>>>>>
> > >>>>>>>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> > >>>>>>>> ---
> > >>>>>>>>  libavcodec/adpcm.c | 2 +-
> > >>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>>>>>
> > >>>>>>>> diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
> > >>>>>>>> index b0c3b91a3b..9993c9e531 100644
> > >>>>>>>> --- a/libavcodec/adpcm.c
> > >>>>>>>> +++ b/libavcodec/adpcm.c
> > >>>>>>>> @@ -1211,7 +1211,7 @@ static int
> > >>> adpcm_decode_frame(AVCodecContext
> > >>>>>>> *avctx, AVFrame *frame,
> > >>>>>>>>
> > >>>>>>>>          for (int i = 0; i < channels; i++) {
> > >>>>>>>>              ADPCMChannelStatus *cs = &c->status[i];
> > >>>>>>>> -            samples = (int16_t *)frame->data[i];
> > >>>>>>>> +            samples = samples_p[i];
> > >>>>>>>>              for (int n = nb_samples >> 1; n > 0; n--) {
> > >>>>>>>>                  int v = bytestream2_get_byteu(&gb);
> > >>>>>>>>                  *samples++ = adpcm_ima_expand_nibble(cs, v &
> > >>> 0x0F,
> > >>>>> 4);
> > >>>>>>>
> > >>>>>>> should be ok if tested
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>> --
> > >>>>>>>> 2.39.1
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>>>  libavcodec/adpcm.c                     |  388
> > >>>>>>> +++++++++++++++++----------------
> > >>>>>>>>  tests/ref/fate/adpcm-creative-8-2.6bit |    2
> > >>>>>>>>  tests/ref/fate/adpcm-creative-8-2bit   |    2
> > >>>>>>>>  tests/ref/fate/adpcm-creative-8-4bit   |    2
> > >>>>>>>>  tests/ref/fate/adpcm-ms-mono           |   60 +----
> > >>>>>>>>  5 files changed, 227 insertions(+), 227 deletions(-)
> > >>>>>>>> 1760df1de66b4227e71ffe942dedcf7d8a33ad48
> > >>>>>>> 0004-avcodec-adpcm-consume-all-input-when-decoding.patch
> > >>>>>>>> From 19789bca53548d672bff30b88a8838edaa876bdb Mon Sep 17
> 00:00:00
> > >>>>> 2001
> > >>>>>>>> From: Paul B Mahol <onemda@gmail.com>
> > >>>>>>>> Date: Tue, 15 Aug 2023 15:25:22 +0200
> > >>>>>>>> Subject: [PATCH 4/4] avcodec/adpcm: consume all input when
> > >>> decoding
> > >>>>>>>>
> > >>>>>>>> Stops multiple decoding calls for single packet.
> > >>>>>>>> Also makes decoding faster.
> > >>>>>>>
> > >>>>>>> This increases latency, which can be problem if packets are
> > >>>>>>> sufficiently large
> > >>>>>>>
> > >>>>>>
> > >>>>>> Then reduce size at demuxer level. there is option for it.
> > >>>>>
> > >>>>> if that is so, then please explain exactly which option should be
> used
> > >>>>> in the commit message
> > >>>>>
> > >>>>>
> > >>>> ffmpeg -h demuxer=wav
> > >>>
> > >>> what about the demuxers that are not named "wav" ?
> > >>>
> > >>>
> > >> I think you are blocking this just for sake of blocking because you
> have
> > >> nothing more constructive to do.
> > >> The delay is always present, unless packet size is exact as block
> align.
> > >> And for ADPCM this is irrelevant.
> > >
> > > IIUC, before this patch if a 1gb sized ADPCM packet comes from a
> demuxer
> > > that is decoded in small kb sized pieces and returned at that
> granularity
> > > to the user.
> > > after this patch (please correct me if iam wrong)
> > > you get a failure as the decoded data is not even addressable with int
> > > but lets assume int is 64bits, you get a 4gb or something like that
> > > audio frame. It maybe faster to decode that in one go but by how much
> is that
> > > faster from lets say decode it in 10-100kb sized chunks ?
> > >
> > > IMHO theres a point where it is too big, and simply returning the data
> > > to the user with some granularity lower than "unlimited" makes sense.
> > > Iam not asking for a new feature, rather your patch removes this
> > >
> >
> > If a user wants his audio in small bits, he should not send 1GB packets.
>
> Normally most users get their audio packets from a file or stream
> they dont choose whats in that and dont choose the size of packets
>
> the wav demuxer has an option to choose the maximum packet size but thats
> wav demuxer specific.
>
> am i missing something ?
> the 1gb was an extreem example to show the issue, real files would
> likely not be that extreem but still the existence of the wav demuxer
> option to choose the maximum packet size shows there was a need for
> limiting it.
>
> And if you take packets from the wav demuxer and remux them into
> something else then any size you can get out of wav, you will be
> able to put in and later get out of any other container.
>
> again am i missing something ?
>

wav demuxer may have set size to higher value than block_align value.
This patch just returns samples in one bigger frame at once if that is
possible.
The delay/latency is not introduced by this patch at all which seems your
only objective.
In fact delay/latency is same as in code without this patch.



>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Into a blind darkness they enter who follow after the Ignorance,
> they as if into a greater darkness enter who devote themselves
> to the Knowledge alone. -- Isha Upanishad
> _______________________________________________
> 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".
>
_______________________________________________
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".