Le decadi 10 thermidor, an CCXXV, Paras Chadha a écrit : > Signed-off-by: Paras Chadha <paraschadh...@gmail.com> > --- > > Made all the changes suggested.
Nice. There are a few nitpicks, but I like these versions much better. > > libavformat/Makefile | 1 + > libavformat/allformats.c | 1 + > libavformat/fitsdec.c | 236 > +++++++++++++++++++++++++++++++++++++++++++++++ > libavformat/version.h | 2 +- > 4 files changed, 239 insertions(+), 1 deletion(-) > create mode 100644 libavformat/fitsdec.c > > diff --git a/libavformat/Makefile b/libavformat/Makefile > index b0ef82c..266b77a 100644 > --- a/libavformat/Makefile > +++ b/libavformat/Makefile > @@ -164,6 +164,7 @@ OBJS-$(CONFIG_FFMETADATA_MUXER) += ffmetaenc.o > OBJS-$(CONFIG_FIFO_MUXER) += fifo.o > OBJS-$(CONFIG_FILMSTRIP_DEMUXER) += filmstripdec.o > OBJS-$(CONFIG_FILMSTRIP_MUXER) += filmstripenc.o > +OBJS-$(CONFIG_FITS_DEMUXER) += fitsdec.o > OBJS-$(CONFIG_FLAC_DEMUXER) += flacdec.o rawdec.o \ > flac_picture.o \ > oggparsevorbis.o \ > diff --git a/libavformat/allformats.c b/libavformat/allformats.c > index 1ebc142..3c12760 100644 > --- a/libavformat/allformats.c > +++ b/libavformat/allformats.c > @@ -121,6 +121,7 @@ static void register_all(void) > REGISTER_MUXDEMUX(FFMETADATA, ffmetadata); > REGISTER_MUXER (FIFO, fifo); > REGISTER_MUXDEMUX(FILMSTRIP, filmstrip); > + REGISTER_DEMUXER (FITS, fits); > REGISTER_MUXDEMUX(FLAC, flac); > REGISTER_DEMUXER (FLIC, flic); > REGISTER_MUXDEMUX(FLV, flv); > diff --git a/libavformat/fitsdec.c b/libavformat/fitsdec.c > new file mode 100644 > index 0000000..a8e9396 > --- /dev/null > +++ b/libavformat/fitsdec.c > @@ -0,0 +1,236 @@ > +/* > + * FITS demuxer > + * Copyright (c) 2017 Paras Chadha > + * > + * 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 > + */ > + > +/** > + * @file > + * FITS demuxer. > + */ > + > +#include "libavutil/intreadwrite.h" > +#include "internal.h" > +#include "libavutil/opt.h" > +#include "libavcodec/fits.h" > +#include "libavutil/bprint.h" > + > +#define FITS_BLOCK_SIZE 2880 > + > +typedef struct FITSContext { > + const AVClass *class; > + AVRational framerate; > + int first_image; > + int image; > + int64_t pts; > +} FITSContext; > + > +static int fits_probe(AVProbeData *p) > +{ > + const uint8_t *b = p->buf; > + > + if (AV_RB64(b) == 0x53494d504c452020 && > + AV_RB64(b + 8) == 0x3D20202020202020 && > + AV_RB64(b + 16) == 0x2020202020202020 && > + AV_RB48(b + 24) == 0x202020202054) See Clément's comment. > + return AVPROBE_SCORE_MAX - 1; > + return 0; > +} > + > +static int fits_read_header(AVFormatContext *s) > +{ > + AVStream *st; > + FITSContext * fits = s->priv_data; > + > + st = avformat_new_stream(s, NULL); > + if (!st) > + return AVERROR(ENOMEM); > + > + st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; > + st->codecpar->codec_id = AV_CODEC_ID_FITS; > + > + avpriv_set_pts_info(st, 64, fits->framerate.den, fits->framerate.num); > + fits->pts = 0; > + fits->first_image = 1; > + return 0; > +} > + > +static int64_t find_size(AVFormatContext *s, FITSContext *fits, FITSHeader > *header, AVBPrint *avbuf) > +{ > + int i, ret; > + char buf[FITS_BLOCK_SIZE] = { 0 }; > + int64_t buf_size = 0, data_size = 0, t; Maybe make them unsigned. > + > + do { > + ret = avio_read(s->pb, buf, FITS_BLOCK_SIZE); > + if (ret < 0) { > + return ret; > + } else if (ret < FITS_BLOCK_SIZE) { > + return AVERROR_EOF; I would say AVERROR_INVALIDDATA for truncated files. > + } > + > + av_bprint_append_data(avbuf, buf, FITS_BLOCK_SIZE); > + ret = 0; > + buf_size = 0; > + while(!ret && buf_size < FITS_BLOCK_SIZE) { > + ret = avpriv_fits_header_parse_line(s, header, buf + buf_size, > NULL); > + buf_size += 80; > + } > + } while (!ret); > + if (ret < 0) > + return ret; > + > + fits->image = fits->first_image || header->image_extension; > + > + if (header->groups) { > + fits->image = 0; > + if (header->naxis > 1) > + data_size = 1; > + for (i = 1; i < header->naxis; i++) { > + if(data_size && header->naxisn[i] > LLONG_MAX / data_size) Use the correct bound, long is wrong. > + return AVERROR_INVALIDDATA; > + data_size *= header->naxisn[i]; > + } > + } else if (header->naxis) { > + data_size = 1; > + for (i = 0; i < header->naxis; i++) { > + if(data_size && header->naxisn[i] > LLONG_MAX / data_size) Ditto. > + return AVERROR_INVALIDDATA; > + data_size *= header->naxisn[i]; > + } > + } else { > + fits->image = 0; > + } > + > + if(header->pcount > LLONG_MAX - data_size) > + return AVERROR_INVALIDDATA; > + data_size += header->pcount; > + > + t = (abs(header->bitpix) >> 3) * ((int64_t) header->gcount); > + if(data_size && t > LLONG_MAX / data_size) Ditto. > + return AVERROR_INVALIDDATA; > + data_size *= t; > + > + if (!data_size) { > + fits->image = 0; > + } else { > + if(2879 > LLONG_MAX - data_size) Ditto. > + return AVERROR_INVALIDDATA; > + data_size = ((data_size + 2879) / 2880) * 2880; > + } > + > + return data_size; > +} > + > +static int fits_read_packet(AVFormatContext *s, AVPacket *pkt) > +{ > + int64_t size=0, pos, ret; > + FITSContext *fits = s->priv_data; > + FITSHeader header; > + AVBPrint avbuf; > + char *buf; > + > + if (fits->first_image) { > + avpriv_fits_header_init(&header, STATE_SIMPLE); > + } else { > + avpriv_fits_header_init(&header, STATE_XTENSION); > + } > + > + fits->image = 0; > + av_bprint_init(&avbuf, FITS_BLOCK_SIZE, AV_BPRINT_SIZE_UNLIMITED); > + size = find_size(s, fits, &header, &avbuf); > + fits->first_image = 0; > + if (size < 0) { > + av_bprint_finalize(&avbuf, NULL); > + return size; > + } I think you could move this block in the loop to have all the init/find_size logic only once. Also, if you make sure avbuf is always inited, you could use a goto to finalize it on error. > + > + while (!fits->image) { > + pos = avio_skip(s->pb, size); > + if (pos < 0) > + return pos; > + > + av_bprint_finalize(&avbuf, NULL); > + av_bprint_init(&avbuf, FITS_BLOCK_SIZE, AV_BPRINT_SIZE_UNLIMITED); > + avpriv_fits_header_init(&header, STATE_XTENSION); > + size = find_size(s, fits, &header, &avbuf); > + if (size < 0) { > + av_bprint_finalize(&avbuf, NULL); > + return size; > + } > + } > + > + if (!av_bprint_is_complete(&avbuf)) { > + av_bprint_finalize(&avbuf, NULL); > + return AVERROR(ENOMEM); > + } > + > + // Header is sent with the first line removed... > + ret = av_new_packet(pkt, avbuf.len - 80 + size); > + if (ret < 0) { > + av_bprint_finalize(&avbuf, NULL); > + return ret; > + } > + pkt->stream_index = 0; > + pkt->flags |= AV_PKT_FLAG_KEY; > + pkt->pos = pos; > + > + ret = av_bprint_finalize(&avbuf, &buf); > + if (ret < 0) { > + av_packet_unref(pkt); > + return ret; > + } > + > + memcpy(pkt->data, buf + 80, avbuf.len - 80); > + pkt->size = avbuf.len - 80; > + av_freep(&buf); > + ret = avio_read(s->pb, pkt->data + pkt->size, size); > + if (ret < 0) { > + av_packet_unref(pkt); > + return ret; > + } > + > + pkt->size += ret; > + pkt->pts = fits->pts; > + fits->pts++; > + > + return 0; > +} > + > +static const AVOption fits_options[] = { > + { "framerate", "set the framerate", offsetof(FITSContext, framerate), > AV_OPT_TYPE_VIDEO_RATE, {.str = "1"}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM}, > + { NULL }, > +}; > + > +static const AVClass fits_demuxer_class = { > + .class_name = "FITS demuxer", > + .item_name = av_default_item_name, > + .option = fits_options, > + .version = LIBAVUTIL_VERSION_INT, > +}; > + > +AVInputFormat ff_fits_demuxer = { > + .name = "fits", > + .long_name = NULL_IF_CONFIG_SMALL("Flexible Image Transport > System"), > + .priv_data_size = sizeof(FITSContext), > + .read_probe = fits_probe, > + .read_header = fits_read_header, > + .read_packet = fits_read_packet, > + .priv_class = &fits_demuxer_class, > + .raw_codec_id = AV_CODEC_ID_FITS, > +}; > diff --git a/libavformat/version.h b/libavformat/version.h > index 48b81f2..a8cf4c1 100644 > --- a/libavformat/version.h > +++ b/libavformat/version.h > @@ -32,7 +32,7 @@ > // 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 57 > -#define LIBAVFORMAT_VERSION_MINOR 76 > +#define LIBAVFORMAT_VERSION_MINOR 77 > #define LIBAVFORMAT_VERSION_MICRO 100 > > #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ No more remarks from me, thanks for humoring my requirements. Regards, -- Nicolas George
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel