Message ID | 20200305004013.29385-3-zane@zanevaniperen.com |
---|---|
State | Superseded |
Headers | show |
Series | High Voltage Software ALP demuxer + decoder. | expand |
Context | Check | Description |
---|---|---|
andriy/ffmpeg-patchwork | success | Make fate finished |
Am 05.03.2020 um 01:40 schrieb Zane van Iperen: > Signed-off-by: Zane van Iperen <zane@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 (zane@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). And why haven't you added a check based upon the "ADPCM" magic word? > + 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. > + > + 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. > + 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. > + } > + > + 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 '='? > + 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. - Andreas
On Thu, 5 Mar 2020 02:26:38 +0100 "Andreas Rheinhardt" <andreas.rheinhardt@gmail.com> wrote: > Am 05.03.2020 um 01:40 schrieb Zane van Iperen: > > Signed-off-by: Zane van Iperen <zane@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 (zane@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-request@ffmpeg.org with subject "unsubscribe". How's this? https://github.com/vs49688/FFmpeg/commit/c43047dac28ac458c2b15e3aa3f5f443c24fc37d.patch Zane
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 (zane@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) + 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)); + + if (!(st = avformat_new_stream(s, NULL))) + 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); + } + + 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; + pkt->stream_index = 0; + pkt->duration = ret * (8 / par->bits_per_coded_sample) / par->channels; + + return 0; +} + +AVInputFormat ff_alp_demuxer = { + .name = "alp", + .long_name = NULL_IF_CONFIG_SMALL("LEGO Racers ALP"), + .read_probe = alp_probe, + .read_header = alp_read_header, + .read_packet = alp_read_packet +}; diff --git a/libavformat/version.h b/libavformat/version.h index 4724269b3c..a233b67351 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -32,8 +32,8 @@ // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium) // Also please add any ticket numbers that you believe might be affected here #define LIBAVFORMAT_VERSION_MAJOR 58 -#define LIBAVFORMAT_VERSION_MINOR 39 -#define LIBAVFORMAT_VERSION_MICRO 101 +#define LIBAVFORMAT_VERSION_MINOR 40 +#define LIBAVFORMAT_VERSION_MICRO 100 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ LIBAVFORMAT_VERSION_MINOR, \
Signed-off-by: Zane van Iperen <zane@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