[FFmpeg-devel] [PATCH] OSQ lossless audio format support
Paul B Mahol
onemda at gmail.com
Thu Aug 24 22:04:55 EEST 2023
On Thu, Aug 24, 2023 at 8:06 PM Michael Niedermayer <michael at niedermayer.cc>
wrote:
> On Thu, Aug 24, 2023 at 11:52:45AM +0200, Paul B Mahol wrote:
> > Patches attached.
> >
> > Stereo decoding have some issues with some predictors so not yet
> bitexact.
> >
> > Please comment.
>
>
> [...]
> > diff --git a/libavformat/osq.c b/libavformat/osq.c
> > new file mode 100644
> > index 0000000000..36ce25313f
> > --- /dev/null
> > +++ b/libavformat/osq.c
> > @@ -0,0 +1,117 @@
> > +/*
> > + * OSQ demuxer
> > + * Copyright (c) 2023 Paul B Mahol
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
> > +#include "libavutil/intreadwrite.h"
> > +#include "avio_internal.h"
> > +#include "avformat.h"
> > +#include "demux.h"
> > +#include "internal.h"
> > +#include "rawdec.h"
> > +
> > +static int osq_probe(const AVProbeData *p)
> > +{
>
> > + if (AV_RL32(p->buf) != MKTAG('O','S','Q',' '))
> > + return 0;
> > + if (AV_RL32(p->buf + 4) != 48)
> > + return 0;
> > + if (AV_RL16(p->buf + 8) != 1)
> > + return 0;
>
> this can be simplified to a single memcmp() with a 10 byte array
>
>
nit
>
> > + if (!p->buf[10])
> > + return 0;
> > + if (!p->buf[11])
> > + return 0;
> > + if (AV_RL32(p->buf + 12) == 0)
> > + return 0;
> > + if (AV_RL16(p->buf + 16) == 0)
> > + return 0;
>
> inconsistant, you mix !x and == 0 for the same kind of check
>
>
nit, changed
>
> > +
> > + return AVPROBE_SCORE_MAX;
> > +}
> > +
> > +static int osq_read_header(AVFormatContext *s)
> > +{
> > + uint32_t t, size;
> > + AVStream *st;
> > + int ret;
> > +
> > + t = avio_rl32(s->pb);
> > + if (t != MKTAG('O','S','Q',' '))
> > + return AVERROR_INVALIDDATA;
> > +
> > + size = avio_rl32(s->pb);
> > + if (size != 48)
> > + return AVERROR_INVALIDDATA;
> > +
> > + st = avformat_new_stream(s, NULL);
> > + if (!st)
> > + return AVERROR(ENOMEM);
> > + if ((ret = ff_get_extradata(s, st->codecpar, s->pb, size)) < 0)
> > + return ret;
> > +
>
> > + t = avio_rl32(s->pb);
> > + if (t != MKTAG('R','I','F','F'))
> > + return AVERROR_INVALIDDATA;
> > + avio_skip(s->pb, 8);
> > +
> > + t = avio_rl32(s->pb);
> > + if (t != MKTAG('f','m','t',' '))
> > + return AVERROR_INVALIDDATA;
> > + size = avio_rl32(s->pb);
> > + avio_skip(s->pb, size);
> > +
> > + t = avio_rl32(s->pb);
> > + size = avio_rl32(s->pb);
> > + while (t != MKTAG('d','a','t','a')) {
> > + avio_skip(s->pb, size);
> > +
> > + t = avio_rl32(s->pb);
> > + size = avio_rl32(s->pb);
> > + if (avio_feof(s->pb))
> > + return AVERROR_INVALIDDATA;
> > + }
>
> This looks familiar, can code be shared with other RIFF based formats ?
>
>
If its really critical, maybe later.
>
> > +
> > + st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
> > + st->codecpar->codec_id = AV_CODEC_ID_OSQ;
> > + st->codecpar->sample_rate = AV_RL32(st->codecpar->extradata + 4);
> > + if (st->codecpar->sample_rate == 0)
> > + return AVERROR_INVALIDDATA;
>
> missing check for negative numbers
>
fixed
>
>
> [...]
> > diff --git a/libavcodec/osq.c b/libavcodec/osq.c
> > new file mode 100644
> > index 0000000000..b6dc5c1bb4
> > --- /dev/null
> > +++ b/libavcodec/osq.c
> > @@ -0,0 +1,435 @@
> > +/*
> > + * OSQ audio decoder
> > + * Copyright (c) 2023 Paul B Mahol
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
>
> > +#define ASSERT_LEVEL 5
>
> that looks strange
> assert level is set by the user building this, also there is no level 5
>
>
removed
>
> > +#include "libavutil/avassert.h"
> > +#include "libavutil/internal.h"
> > +#include "libavutil/intreadwrite.h"
> > +#include "avcodec.h"
> > +#include "codec_internal.h"
> > +#include "decode.h"
> > +#include "internal.h"
> > +#define BITSTREAM_READER_LE
> > +#include "get_bits.h"
> > +#include "unary.h"
> > +
> > +typedef struct OSQChannel {
> > + int prediction;
> > + int coding_mode;
> > + int residue_parameter;
> > + int residue_bits;
> > +} OSQChannel;
> > +
> > +typedef struct OSQContext {
> > + GetBitContext gb;
> > + OSQChannel ch[2];
> > +
> > + uint8_t *bitstream;
>
> > + int64_t max_framesize;
>
> i think the max_framesize fits in 29bits currently so 64 should not be
> needed
>
>
Even if that is true, argument to function is size_t, so changed to that
one.
>
> [...]
>
>
> > +
> > + s->bitstream = av_calloc(s->max_framesize +
> AV_INPUT_BUFFER_PADDING_SIZE, sizeof(*s->bitstream));
> > + if (!s->bitstream)
> > + return AVERROR(ENOMEM);
> > +
> > + for (int ch = 0; ch < avctx->ch_layout.nb_channels; ch++) {
> > + s->decode_buffer[ch] = av_calloc(s->frame_samples + 4,
> > + sizeof(*s->decode_buffer[ch]));
> > + if (!s->decode_buffer[ch])
> > + return AVERROR(ENOMEM);
> > + }
> > +
> > + s->pkt = avctx->internal->in_pkt;
> > +
> > + return 0;
> > +}
> > +
> > +static uint32_t get_urice(GetBitContext *gb, int k)
> > +{
> > + uint32_t z, x, b;
> > +
> > + x = get_unary(gb, 1, 512);
> > + b = get_bits_long(gb, k & 31);
> > + z = b | x << (k & 31);
>
> the k & 31 seems unneeded
>
Removed
>
>
> [...]
> > + }
> > + break;
> > + case AV_SAMPLE_FMT_S32P:
> > + for (int ch = 0; ch < avctx->ch_layout.nb_channels; ch++) {
> > + int32_t *dst = (int32_t *)frame->extended_data[ch];
> > + int32_t *src = s->decode_buffer[ch] + 4;
> > +
> > + for (int n = 0; n < frame->nb_samples; n++)
> > + dst[n] = src[n];
>
> memcpy() unless these can overlap
>
>
bit depth is not always 32 and thus not going into that direction.
But >16bit need to be yet tested.
> thx
>
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> While the State exists there can be no freedom; when there is freedom there
> will be no State. -- Vladimir Lenin
> _______________________________________________
> 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