Re: [FFmpeg-devel] [PATCH] QOA decoding support

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

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).

> 
> +#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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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