[FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer
Stefano Sabatini
stefasab at gmail.com
Sat Dec 10 18:55:53 EET 2016
On date Sunday 2016-12-04 22:54:21 +0100, Andreas Cadhalpun encoded:
> On 31.10.2016 09:51, Stefano Sabatini wrote:
> > From 7f209e27aa33e8f43444e5cfc44c68f664b69e06 Mon Sep 17 00:00:00 2001
> > From: Nicolas George <george at nsup.org>
> > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > Subject: [PATCH] lavf: add ffprobe demuxer
> >
> > With several modifications and documentation by Stefano Sabatini
> > <stefasab at gmail.com>.
> >
> > Signed-off-by: Nicolas George <george at nsup.org>
> > ---
> > configure | 3 +
> > doc/demuxers.texi | 18 ++
> > doc/ffprobe-format.texi | 121 +++++++++++++
> > doc/formats.texi | 1 +
> > libavformat/Makefile | 1 +
> > libavformat/allformats.c | 1 +
> > libavformat/ffprobedec.c | 439 +++++++++++++++++++++++++++++++++++++++++++++++
> > 7 files changed, 584 insertions(+)
> > create mode 100644 doc/ffprobe-format.texi
> > create mode 100644 libavformat/ffprobedec.c
> >
> > diff --git a/configure b/configure
> > index 72ffaea..71b9d73 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3349,6 +3349,9 @@ for n in $COMPONENT_LIST; do
> > eval ${n}_if_any="\$$v"
> > done
> >
> > +# Disable ffprobe demuxer for security concerns
> > +disable ffprobe_demuxer
> > +
>
> As I already wrote elsewhere, I don't think disabling this by default is good,
> as it will likely cause it to bitrot. Better require '-strict experimental'.
>
> > enable $ARCH_EXT_LIST
> >
> > die_unknown(){
> > diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> > index 2934a1c..0e6dfbd 100644
> > --- a/doc/demuxers.texi
> > +++ b/doc/demuxers.texi
> > @@ -243,6 +243,24 @@ file subdir/file-2.wav
> > @end example
> > @end itemize
> >
> > + at section ffprobe
> > +
> > +ffprobe internal demuxer. It is able to demux files in the format
> > +specified in the @ref{FFprobe demuxer format} chapter.
> > +
> > +For security reasons this demuxer is disabled by default, should be
> > +enabled though the @code{--enable-demuxer=ffprobe} configure option.
> > +
>
> In that case the documentation should mention '-strict experimental'
> instead of this.
>
> > diff --git a/libavformat/ffprobedec.c b/libavformat/ffprobedec.c
> > new file mode 100644
> > index 0000000..0bc9a49
> > --- /dev/null
> > +++ b/libavformat/ffprobedec.c
> [...]
> > +static int read_section_stream(AVFormatContext *avf)
> > +{
> > + FFprobeContext *ffp = avf->priv_data;
> > + uint8_t buf[LINE_BUFFER_SIZE];
> > + int ret, index, val1, val2;
> > + AVStream *st = NULL;
> > + const char *val;
> > +
> > + av_bprint_clear(&ffp->data);
> > + while ((ret = read_section_line(avf, buf, sizeof(buf)))) {
> > + if (ret < 0)
> > + return ret;
> > + if (!st) {
> > + if (sscanf(buf, "index=%d", &index) >= 1) {
> > + if (index == avf->nb_streams) {
> > + if (!avformat_new_stream(avf, NULL))
> > + return AVERROR(ENOMEM);
> > + }
> > + if ((unsigned)index >= avf->nb_streams) {
> > + av_log(avf, AV_LOG_ERROR, "Invalid stream index: %d\n",
> > + index);
> > + return AVERROR_INVALIDDATA;
> > + }
> > + st = avf->streams[index];
> > + } else {
> > + av_log(avf, AV_LOG_ERROR, "Stream without index\n");
> > + return AVERROR_INVALIDDATA;
> > + }
> > + }
> > + if (av_strstart(buf, "codec_name=", &val)) {
> > + const AVCodecDescriptor *desc = avcodec_descriptor_get_by_name(val);
> > + if (desc) {
> > + st->codecpar->codec_id = desc->id;
> > + st->codecpar->codec_type = desc->type;
> > + }
> > + if (!desc) {
> > + av_log(avf, AV_LOG_WARNING, "Cannot recognize codec name '%s'", val);
> > + }
> > + } else if (!strcmp(buf, "extradata=")) {
> > + if ((ret = read_data(avf, NULL)) < 0)
> > + return ret;
> > + if (ffp->data.len) {
>
> This can leak st->codecpar->extradata and thus needs:
> av_freep(&st->codecpar->extradata);
>
> > + if ((ret = ff_alloc_extradata(st->codecpar, ffp->data.len)) < 0)
> > + return ret;
> > + memcpy(st->codecpar->extradata, ffp->data.str, ffp->data.len);
> > + }
> > + } else if (sscanf(buf, "time_base=%d/%d", &val1, &val2) >= 2) {
> > + st->time_base.num = val1;
> > + st->time_base.den = val2;
> > + if (st->time_base.num <= 0 || st->time_base.den <= 0) {
>
> This should check val1 and val2 and only afterwards set st->time_base.
> Otherwise the check doesn't have the desired effect.
No, this doesn't make any difference but changed anyway as it reduces
the character count.
> > + av_log(avf, AV_LOG_ERROR, "Invalid time base %d/%d\n",
> > + st->time_base.num, st->time_base.den);
> > + return AVERROR_INVALIDDATA;
> > + }
> > + }
> > + }
> > + return SEC_STREAM;
> > +}
> > +
> > +static int read_section_packet(AVFormatContext *avf, AVPacket *pkt)
> > +{
> > + FFprobeContext *ffp = avf->priv_data;
> > + uint8_t buf[LINE_BUFFER_SIZE];
> > + int ret;
> > + AVPacket p;
> > + char flags;
> > + int has_stream_index = 0;
> > + int64_t pts, dts, duration;
>
> These three variables need to be initialized to prevent use of uninitialized memory.
>
> > +
> > + av_init_packet(&p);
> > + p.pos = avio_tell(avf->pb);
> > + p.stream_index = -1;
> > + p.size = -1;
> > + av_bprint_clear(&ffp->data);
> > +
> > + while ((ret = read_section_line(avf, buf, sizeof(buf)))) {
> > + int has_time = 0;
> > + char timebuf[LINE_BUFFER_SIZE];
>
> This buffer needs to be initialized, as well.
[...]
> > +static int ffprobe_read_header(AVFormatContext *avf)
> > +{
> > + FFprobeContext *ffp = avf->priv_data;
> > + int ret;
> > + int nb_streams = 0;
> > +
>
> I suggest to add here something like:
> if (avf->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) {
> av_log(avf, AV_LOG_ERROR, "The ffprobe demuxer is experimental and requires '-strict experimental'.\n");
> return AVERROR_EXPERIMENTAL;
> }
>
> Otherwise this patch looks good to me.
Updated, thanks.
--
FFmpeg = Freak Frenzy Magical Puristic Embarassing Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-lavf-add-ffprobe-demuxer.patch
Type: text/x-diff
Size: 21563 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161210/4912b73c/attachment.patch>
More information about the ffmpeg-devel
mailing list