On Thu, 5 Mar 2020 02:26:38 +0100
"Andreas Rheinhardt" <andreas.rheinha...@gmail.com> wrote:
> Am 05.03.2020 um 01:40 schrieb Zane van Iperen:
> > Signed-off-by: Zane van Iperen <z...@zanevaniperen.com>
> > ---
> > libavformat/Makefile | 1 +
> > libavformat/allformats.c | 1 +
> > libavformat/alp.c | 135
> > +++++++++++++++++++++++++++++++++++++++ libavformat/version.h
> > | 4 +- 4 files changed, 139 insertions(+), 2 deletions(-)
> > create mode 100644 libavformat/alp.c
> >
> > diff --git a/libavformat/Makefile b/libavformat/Makefile
> > index e0681058a2..fbb29505ff 100644
> > --- a/libavformat/Makefile
> > +++ b/libavformat/Makefile
> > @@ -85,6 +85,7 @@ OBJS-$(CONFIG_AIFF_DEMUXER) +=
> > aiffdec.o pcm.o isom.o \ mov_chan.o replaygain.o
> > OBJS-$(CONFIG_AIFF_MUXER) += aiffenc.o id3v2enc.o
> > OBJS-$(CONFIG_AIX_DEMUXER) += aixdec.o
> > +OBJS-$(CONFIG_ALP_DEMUXER) += alp.o
> > OBJS-$(CONFIG_AMR_DEMUXER) += amr.o
> > OBJS-$(CONFIG_AMR_MUXER) += amr.o
> > OBJS-$(CONFIG_AMRNB_DEMUXER) += amr.o
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> > index 0209bf0e30..08012ea208 100644
> > --- a/libavformat/allformats.c
> > +++ b/libavformat/allformats.c
> > @@ -46,6 +46,7 @@ extern AVInputFormat ff_afc_demuxer;
> > extern AVInputFormat ff_aiff_demuxer;
> > extern AVOutputFormat ff_aiff_muxer;
> > extern AVInputFormat ff_aix_demuxer;
> > +extern AVInputFormat ff_alp_demuxer;
> > extern AVInputFormat ff_amr_demuxer;
> > extern AVOutputFormat ff_amr_muxer;
> > extern AVInputFormat ff_amrnb_demuxer;
> > diff --git a/libavformat/alp.c b/libavformat/alp.c
> > new file mode 100644
> > index 0000000000..76703e7ad1
> > --- /dev/null
> > +++ b/libavformat/alp.c
> > @@ -0,0 +1,135 @@
> > +/*
> > + * LEGO Racers ALP (.tun & .pcm) demuxer
> > + *
> > + * Copyright (C) 2020 Zane van Iperen (z...@zanevaniperen.com)
> > + *
> > + * 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 "avformat.h"
> > +#include "internal.h"
> > +#include "libavutil/intreadwrite.h"
> > +
> > +#define ALP_TAG MKTAG('A', 'L', 'P', ' ')
> > +#define ALP_MAX_READ_SIZE 4096
> > +
> > +typedef struct ALPHeader {
> > + uint32_t magic; /*< Magic Number, {'A', 'L', 'P',
> > ' '} */
> > + uint32_t header_size; /*< Header size (after this). */
> > + char adpcm[6]; /*< "ADPCM" */
> > + uint8_t unk1; /*< Unknown */
> > + uint8_t num_channels; /*< Channel Count. */
> > + uint32_t sample_rate; /*< Sample rate, only if
> > header_size >= 12. */ +} ALPHeader;
> > +
> > +static int alp_probe(const AVProbeData *p)
> > +{
> > + if (AV_RL32(p->buf) != ALP_TAG)
> > + return 0;
> > +
> > + if (AV_RL32(p->buf) < 8)
>
> This check must be wrong, because you already know that the left side
> here is ALP_TAG. Did you mean AV_RL32(p->buf + 4)? If so, you should
> explicitly check for it being 8 or 12 (because otherwise the demuxer
> would just return AVERROR_INVALIDDATA).
Yep, I meant +4. Fixed.
>
> And why haven't you added a check based upon the "ADPCM" magic word?
>
...That is a good question, my bad.
> > + return 0;
> > +
> > + return AVPROBE_SCORE_EXTENSION + 1;
> > +}
> > +
> > +static int alp_read_header(AVFormatContext *s)
> > +{
> > + int ret;
> > + AVStream *st;
> > + ALPHeader hdr;
> > + AVCodecParameters *par;
> > +
> > + memset(&hdr, 0, sizeof(hdr));
>
> You actually set every field you use before you use it, so this is
> unnecessary.
>
Better safe then sorry. I've removed it.
> > +
> > + if (!(st = avformat_new_stream(s, NULL)))
>
> This could be moved down below so that you avoid allocating an
> AVStream if you error out later.
>
Done.
> > + return AVERROR(ENOMEM);
> > +
> > + if ((hdr.magic = avio_rl32(s->pb)) != ALP_TAG)
> > + return AVERROR_INVALIDDATA;
> > +
> > + hdr.header_size = avio_rl32(s->pb);
> > +
> > + if (hdr.header_size != 8 && hdr.header_size != 12) {
> > + return AVERROR_INVALIDDATA;
> > + }
> > +
> > + if ((ret = avio_read(s->pb, hdr.adpcm, sizeof(hdr.adpcm))) < 0)
> > + return ret;
> > + else if (ret != sizeof(hdr.adpcm))
> > + return AVERROR(EIO);
> > +
> > + if (strncmp("ADPCM", hdr.adpcm, sizeof(hdr.adpcm)) != 0)
> > + return AVERROR_INVALIDDATA;
> > +
> > + hdr.unk1 = avio_r8(s->pb);
> > + hdr.num_channels = avio_r8(s->pb);
> > +
> > + if (hdr.header_size == 8) {
> > + /* .TUN music file */
> > + hdr.sample_rate = 11025 * hdr.num_channels;
> > + } else {
> > + /* .PCM sound file */
> > + hdr.sample_rate = avio_rl32(s->pb);
>
> If you don't add a check for this, the multiplication for the bit_rate
> below can overflow and par->sample_rate can end up negative.
>
What am I checking for? Are you suggesting something like an upper
limit on hdr.sample_rate?
If so, 44100 ought to do it. I've never seen files above that.
> > + }
> > +
> > + par = st->codecpar;
> > + par->codec_type = AVMEDIA_TYPE_AUDIO;
> > + par->codec_id = AV_CODEC_ID_ADPCM_IMA_ALP;
> > + par->format = AV_SAMPLE_FMT_S16;
> > + par->sample_rate = hdr.sample_rate;
> > + par->channels = hdr.num_channels;
> > +
> > + if (hdr.num_channels == 1)
> > + par->channel_layout = AV_CH_LAYOUT_MONO;
> > + else if (hdr.num_channels == 2)
> > + par->channel_layout = AV_CH_LAYOUT_STEREO;
> > + else
> > + return AVERROR_INVALIDDATA;
> > +
> > + par->bits_per_coded_sample = 4;
> > + par->bits_per_raw_sample = 16;
> > + par->block_align = 1;
> > + par->bit_rate = par->channels *
> > + par->sample_rate *
> > + par->bits_per_coded_sample;
> > +
> > + avpriv_set_pts_info(st, 64, 1, par->sample_rate);
> > + return 0;
> > +}
> > +
> > +static int alp_read_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > + int ret;
> > + AVCodecParameters *par = s->streams[0]->codecpar;
> > +
> > + if ((ret = av_get_packet(s->pb, pkt, ALP_MAX_READ_SIZE)) < 0)
> > + return ret;
> > +
> > + pkt->flags &= ~AV_PKT_FLAG_CORRUPT;
>
> Why don't you align on the '='?
>
Fixed.
> > + pkt->stream_index = 0;
> > + pkt->duration = ret * (8 / par->bits_per_coded_sample) /
> > par->channels;
>
> If par->bits_per_coded_sample is always four, you could hardcode this
> here.
Fixed.
>
> - 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-requ...@ffmpeg.org with subject "unsubscribe".
How's this?
https://github.com/vs49688/FFmpeg/commit/c43047dac28ac458c2b15e3aa3f5f443c24fc37d.patch
Zane
_______________________________________________
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".
