On Thu, Aug 24, 2023 at 8:06 PM Michael Niedermayer <mich...@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".