| Message ID | 20200314115601.24345-2-onemda@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec: add ADPCM IMA MTF decoder | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Paul B Mahol: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavformat/Makefile | 1 + > libavformat/allformats.c | 1 + > libavformat/fwse.c | 88 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 90 insertions(+) > create mode 100644 libavformat/fwse.c > > diff --git a/libavformat/Makefile b/libavformat/Makefile > index f84becd30a..cd3e9163f5 100644 > --- a/libavformat/Makefile > +++ b/libavformat/Makefile > @@ -198,6 +198,7 @@ OBJS-$(CONFIG_FRAMEHASH_MUXER) += hashenc.o framehash.o > OBJS-$(CONFIG_FRAMEMD5_MUXER) += hashenc.o framehash.o > OBJS-$(CONFIG_FRM_DEMUXER) += frmdec.o > OBJS-$(CONFIG_FSB_DEMUXER) += fsb.o > +OBJS-$(CONFIG_FWSE_DEMUXER) += fwse.o > OBJS-$(CONFIG_GIF_MUXER) += gif.o > OBJS-$(CONFIG_GIF_DEMUXER) += gifdec.o > OBJS-$(CONFIG_GSM_DEMUXER) += gsmdec.o > diff --git a/libavformat/allformats.c b/libavformat/allformats.c > index 08012ea208..d275c1017b 100644 > --- a/libavformat/allformats.c > +++ b/libavformat/allformats.c > @@ -156,6 +156,7 @@ extern AVOutputFormat ff_framehash_muxer; > extern AVOutputFormat ff_framemd5_muxer; > extern AVInputFormat ff_frm_demuxer; > extern AVInputFormat ff_fsb_demuxer; > +extern AVInputFormat ff_fwse_demuxer; > extern AVInputFormat ff_g722_demuxer; > extern AVOutputFormat ff_g722_muxer; > extern AVInputFormat ff_g723_1_demuxer; > diff --git a/libavformat/fwse.c b/libavformat/fwse.c > new file mode 100644 > index 0000000000..76ed6b0c23 > --- /dev/null > +++ b/libavformat/fwse.c > @@ -0,0 +1,88 @@ > +/* > + * FWSE demuxer > + * Copyright (c) 2020 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 "libavcodec/internal.h" What are you using from this header? > +#include "avformat.h" > +#include "internal.h" > +#include "pcm.h" > + > +typedef struct FWSEDemuxContext { > + unsigned dsp_int_type; > + unsigned interleave_size; > +} FWSEDemuxContext; This structure is completely unused. > + > +static int fwse_probe(const AVProbeData *p) > +{ > + if (AV_RL32(p->buf) != MKTAG('F','W','S','E')) > + return 0; > + if (AV_RL32(p->buf+4) != 2 && AV_RL32(p->buf+4) != 3) > + return 0; > + > + return AVPROBE_SCORE_MAX / 3 * 2; > +} > + > +static int fwse_read_header(AVFormatContext *s) > +{ > + FWSEDemuxContext *c = s->priv_data; Doesn't your compiler warn you about unused variables like this one? > + unsigned start_offset, version; > + AVStream *st; > + > + avio_skip(s->pb, 4); You're checking the version, yet you are not checking the tag? > + version = avio_rl32(s->pb); > + if (version != 1 && version != 2) fwse_probe wants 2 or 3 and here it is 1 or 2. > + return AVERROR_INVALIDDATA; > + avio_skip(s->pb, 4); > + start_offset = avio_rl32(s->pb); > + > + st = avformat_new_stream(s, NULL); > + if (!st) > + return AVERROR(ENOMEM); > + > + st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; Why don't you access codecpar via its own pointer to reduce the amount of writing? (You could do the same with s->pb.) > + st->codecpar->codec_id = AV_CODEC_ID_ADPCM_IMA_MTF; > + st->codecpar->format = AV_SAMPLE_FMT_S16; > + st->codecpar->channels = avio_rl32(s->pb); > + if (st->codecpar->channels != 1 && st->codecpar->channels != 2)> + return AVERROR_INVALIDDATA; Why don't you just add this as another else at the end of the following checks? (And why don't you check for this during probing?) > + if (st->codecpar->channels == 1) > + st->codecpar->channel_layout = AV_CH_LAYOUT_MONO; > + else if (st->codecpar->channels == 2) > + st->codecpar->channel_layout = AV_CH_LAYOUT_STEREO; > + st->duration = avio_rl32(s->pb); > + st->codecpar->sample_rate = avio_rl32(s->pb); No check for insane values? > + st->codecpar->block_align = 1; > + avio_skip(s->pb, start_offset - avio_tell(s->pb)); avio_seek(s->pb, start_offset, SEEK_SET)? > + > + avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); > + > + return 0; > +} > + > +AVInputFormat ff_fwse_demuxer = { > + .name = "fwse", > + .long_name = NULL_IF_CONFIG_SMALL("Capcom's MT Framework sound"), > + .priv_data_size = sizeof(FWSEDemuxContext), > + .read_probe = fwse_probe, > + .read_header = fwse_read_header, > + .read_packet = ff_pcm_read_packet, > + .extensions = "fwse", > +}; >
On 3/14/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote: > Paul B Mahol: >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavformat/Makefile | 1 + >> libavformat/allformats.c | 1 + >> libavformat/fwse.c | 88 ++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 90 insertions(+) >> create mode 100644 libavformat/fwse.c >> >> diff --git a/libavformat/Makefile b/libavformat/Makefile >> index f84becd30a..cd3e9163f5 100644 >> --- a/libavformat/Makefile >> +++ b/libavformat/Makefile >> @@ -198,6 +198,7 @@ OBJS-$(CONFIG_FRAMEHASH_MUXER) += hashenc.o >> framehash.o >> OBJS-$(CONFIG_FRAMEMD5_MUXER) += hashenc.o framehash.o >> OBJS-$(CONFIG_FRM_DEMUXER) += frmdec.o >> OBJS-$(CONFIG_FSB_DEMUXER) += fsb.o >> +OBJS-$(CONFIG_FWSE_DEMUXER) += fwse.o >> OBJS-$(CONFIG_GIF_MUXER) += gif.o >> OBJS-$(CONFIG_GIF_DEMUXER) += gifdec.o >> OBJS-$(CONFIG_GSM_DEMUXER) += gsmdec.o >> diff --git a/libavformat/allformats.c b/libavformat/allformats.c >> index 08012ea208..d275c1017b 100644 >> --- a/libavformat/allformats.c >> +++ b/libavformat/allformats.c >> @@ -156,6 +156,7 @@ extern AVOutputFormat ff_framehash_muxer; >> extern AVOutputFormat ff_framemd5_muxer; >> extern AVInputFormat ff_frm_demuxer; >> extern AVInputFormat ff_fsb_demuxer; >> +extern AVInputFormat ff_fwse_demuxer; >> extern AVInputFormat ff_g722_demuxer; >> extern AVOutputFormat ff_g722_muxer; >> extern AVInputFormat ff_g723_1_demuxer; >> diff --git a/libavformat/fwse.c b/libavformat/fwse.c >> new file mode 100644 >> index 0000000000..76ed6b0c23 >> --- /dev/null >> +++ b/libavformat/fwse.c >> @@ -0,0 +1,88 @@ >> +/* >> + * FWSE demuxer >> + * Copyright (c) 2020 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 "libavcodec/internal.h" > > What are you using from this header? Will remove. > >> +#include "avformat.h" >> +#include "internal.h" >> +#include "pcm.h" >> + >> +typedef struct FWSEDemuxContext { >> + unsigned dsp_int_type; >> + unsigned interleave_size; >> +} FWSEDemuxContext; > > This structure is completely unused. Will remove. >> + >> +static int fwse_probe(const AVProbeData *p) >> +{ >> + if (AV_RL32(p->buf) != MKTAG('F','W','S','E')) >> + return 0; >> + if (AV_RL32(p->buf+4) != 2 && AV_RL32(p->buf+4) != 3) >> + return 0; >> + >> + return AVPROBE_SCORE_MAX / 3 * 2; >> +} >> + >> +static int fwse_read_header(AVFormatContext *s) >> +{ >> + FWSEDemuxContext *c = s->priv_data; > > Doesn't your compiler warn you about unused variables like this one? Will remove. > >> + unsigned start_offset, version; >> + AVStream *st; >> + >> + avio_skip(s->pb, 4); > > You're checking the version, yet you are not checking the tag? That is valid case, if somebody forces demuxer. > >> + version = avio_rl32(s->pb); >> + if (version != 1 && version != 2) > > fwse_probe wants 2 or 3 and here it is 1 or 2. Fill fix. > >> + return AVERROR_INVALIDDATA; >> + avio_skip(s->pb, 4); >> + start_offset = avio_rl32(s->pb); >> + >> + st = avformat_new_stream(s, NULL); >> + if (!st) >> + return AVERROR(ENOMEM); >> + >> + st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; > > Why don't you access codecpar via its own pointer to reduce the amount > of writing? (You could do the same with s->pb.) Why not. > >> + st->codecpar->codec_id = AV_CODEC_ID_ADPCM_IMA_MTF; >> + st->codecpar->format = AV_SAMPLE_FMT_S16; >> + st->codecpar->channels = avio_rl32(s->pb); >> + if (st->codecpar->channels != 1 && st->codecpar->channels != 2)> + >> return AVERROR_INVALIDDATA; > > Why don't you just add this as another else at the end of the following > checks? (And why don't you check for this during probing?) Why not. > >> + if (st->codecpar->channels == 1) >> + st->codecpar->channel_layout = AV_CH_LAYOUT_MONO; >> + else if (st->codecpar->channels == 2) >> + st->codecpar->channel_layout = AV_CH_LAYOUT_STEREO; >> + st->duration = avio_rl32(s->pb); >> + st->codecpar->sample_rate = avio_rl32(s->pb); > > No check for insane values? Not demuxer job. > >> + st->codecpar->block_align = 1; >> + avio_skip(s->pb, start_offset - avio_tell(s->pb)); > > avio_seek(s->pb, start_offset, SEEK_SET)? I prefer skip. >> + >> + avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); >> + >> + return 0; >> +} >> + >> +AVInputFormat ff_fwse_demuxer = { >> + .name = "fwse", >> + .long_name = NULL_IF_CONFIG_SMALL("Capcom's MT Framework >> sound"), >> + .priv_data_size = sizeof(FWSEDemuxContext), >> + .read_probe = fwse_probe, >> + .read_header = fwse_read_header, >> + .read_packet = ff_pcm_read_packet, >> + .extensions = "fwse", >> +}; >> > > _______________________________________________ > 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".
diff --git a/libavformat/Makefile b/libavformat/Makefile index f84becd30a..cd3e9163f5 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -198,6 +198,7 @@ OBJS-$(CONFIG_FRAMEHASH_MUXER) += hashenc.o framehash.o OBJS-$(CONFIG_FRAMEMD5_MUXER) += hashenc.o framehash.o OBJS-$(CONFIG_FRM_DEMUXER) += frmdec.o OBJS-$(CONFIG_FSB_DEMUXER) += fsb.o +OBJS-$(CONFIG_FWSE_DEMUXER) += fwse.o OBJS-$(CONFIG_GIF_MUXER) += gif.o OBJS-$(CONFIG_GIF_DEMUXER) += gifdec.o OBJS-$(CONFIG_GSM_DEMUXER) += gsmdec.o diff --git a/libavformat/allformats.c b/libavformat/allformats.c index 08012ea208..d275c1017b 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -156,6 +156,7 @@ extern AVOutputFormat ff_framehash_muxer; extern AVOutputFormat ff_framemd5_muxer; extern AVInputFormat ff_frm_demuxer; extern AVInputFormat ff_fsb_demuxer; +extern AVInputFormat ff_fwse_demuxer; extern AVInputFormat ff_g722_demuxer; extern AVOutputFormat ff_g722_muxer; extern AVInputFormat ff_g723_1_demuxer; diff --git a/libavformat/fwse.c b/libavformat/fwse.c new file mode 100644 index 0000000000..76ed6b0c23 --- /dev/null +++ b/libavformat/fwse.c @@ -0,0 +1,88 @@ +/* + * FWSE demuxer + * Copyright (c) 2020 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 "libavcodec/internal.h" +#include "avformat.h" +#include "internal.h" +#include "pcm.h" + +typedef struct FWSEDemuxContext { + unsigned dsp_int_type; + unsigned interleave_size; +} FWSEDemuxContext; + +static int fwse_probe(const AVProbeData *p) +{ + if (AV_RL32(p->buf) != MKTAG('F','W','S','E')) + return 0; + if (AV_RL32(p->buf+4) != 2 && AV_RL32(p->buf+4) != 3) + return 0; + + return AVPROBE_SCORE_MAX / 3 * 2; +} + +static int fwse_read_header(AVFormatContext *s) +{ + FWSEDemuxContext *c = s->priv_data; + unsigned start_offset, version; + AVStream *st; + + avio_skip(s->pb, 4); + version = avio_rl32(s->pb); + if (version != 1 && version != 2) + return AVERROR_INVALIDDATA; + avio_skip(s->pb, 4); + start_offset = avio_rl32(s->pb); + + st = avformat_new_stream(s, NULL); + if (!st) + return AVERROR(ENOMEM); + + st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; + st->codecpar->codec_id = AV_CODEC_ID_ADPCM_IMA_MTF; + st->codecpar->format = AV_SAMPLE_FMT_S16; + st->codecpar->channels = avio_rl32(s->pb); + if (st->codecpar->channels != 1 && st->codecpar->channels != 2) + return AVERROR_INVALIDDATA; + if (st->codecpar->channels == 1) + st->codecpar->channel_layout = AV_CH_LAYOUT_MONO; + else if (st->codecpar->channels == 2) + st->codecpar->channel_layout = AV_CH_LAYOUT_STEREO; + st->duration = avio_rl32(s->pb); + st->codecpar->sample_rate = avio_rl32(s->pb); + st->codecpar->block_align = 1; + avio_skip(s->pb, start_offset - avio_tell(s->pb)); + + avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); + + return 0; +} + +AVInputFormat ff_fwse_demuxer = { + .name = "fwse", + .long_name = NULL_IF_CONFIG_SMALL("Capcom's MT Framework sound"), + .priv_data_size = sizeof(FWSEDemuxContext), + .read_probe = fwse_probe, + .read_header = fwse_read_header, + .read_packet = ff_pcm_read_packet, + .extensions = "fwse", +};
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavformat/Makefile | 1 + libavformat/allformats.c | 1 + libavformat/fwse.c | 88 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 libavformat/fwse.c