[FFmpeg-devel] [PATCH] QOA decoding support

Paul B Mahol onemda at gmail.com
Sun Sep 24 03:48:49 EEST 2023


On Sun, Sep 24, 2023 at 2:04 AM Andreas Rheinhardt <
andreas.rheinhardt at outlook.com> wrote:

> Paul B Mahol:
> > +    .flags          = AVFMT_GENERIC_INDEX,
> > +    .extensions     = "qoa",
> > +    .raw_codec_id   = AV_CODEC_ID_QOA,
>
> This will not compile: The codec_id is only added in the second patch.
>
> > +    .priv_data_size = sizeof(FFRawDemuxerContext),
> > +    .priv_class     = &ff_raw_demuxer_class,
>
>
> >
> > +        if (!qoa->frame_size) {
> > +            for (; i < buf_size; i++) {
> > +                state = (state << 8) | buf[i];
> > +                if (((state & 0xFFFF) > 0 && (state >> 56))) {
> > +                    qoa->frame_size = state & 0xFFFF;
> > +                    qoa->duration = (state >> 16) & 0xFFFF;
> > +                    break;
> > +                }
> > +            }
> > +        }
>
> So this codec uses a length field. In this case it is quite simple to
> avoid the parser (and its implicit memcpy) altogether and just make the
> demuxer directly output packets of the correct size. This is quite
> natural given that this format does not seem to provide any features
> like resyncing support (or at least the parser does not implement them).
>

But channels/sample rate may differ between packets.
Also it may be in other formats, like wav. So I picked parser as more
valuable implementation.


>
> >
> > +#include "avcodec.h"
> > +#include "codec_internal.h"
> > +#include "decode.h"
> > +#include "get_bits.h"
>
> You don't use the GetBit API at all.
>
> > +#include "bytestream.h"
>
> >
> > +    for (int sample_index = 0; sample_index < frame->nb_samples *
> nb_channels;
> > +         sample_index += QOA_SLICE_LEN) {
> > +        for (int ch = 0; ch < nb_channels; ch++) {
>
> So the number of times the second loop is executed is quadratic in
> nb_channels. Is this really intended? The frame_size check is not
> quadratic in nb_channels. In fact, it does not seem to account for this
> double-loop here at all.
>
> - Andreas
>
> _______________________________________________
> 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".
>


More information about the ffmpeg-devel mailing list