| Message ID | 20161031085110.GC6666@barisone |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, 31 Oct 2016 09:51:10 +0100 Stefano Sabatini <stefasab@gmail.com> wrote: > On date Thursday 2016-10-27 17:08:50 +0200, Stefano Sabatini encoded: > > On date Wednesday 2016-10-26 01:46:35 +0200, Michael Niedermayer encoded: > > > On Tue, Oct 25, 2016 at 05:07:34PM +0200, Stefano Sabatini wrote: > > [...] > > > > 267580c51d49daf94e73a33175c63ecfed6a0bed 0002-lavf-add-ffprobe-demuxer.patch > > > > From 4e0aac4bc00104483859f9950af2ffb15fea6c12 Mon Sep 17 00:00:00 2001 > > > > From: Nicolas George <george@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@gmail.com>. > > > > > > > > Signed-off-by: Nicolas George <george@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 | 405 +++++++++++++++++++++++++++++++++++++++++++++++ > > > > 7 files changed, 550 insertions(+) > > > > create mode 100644 doc/ffprobe-format.texi > > > > create mode 100644 libavformat/ffprobedec.c > > > > > > did you test this with some fuzzer ? > > > > > > no further comments from me > > > Acked-by: Michael > > > > > Good idea, found a few crashes with afl. I'll let it run again and > > fix found issues. Then I'll push in a few days if there are no more > > comments. > > Fixed a few hangs and crashes found with afl. Will try also to add a > fate test. You still didn't add my Nacked-by header. I very strongly disagree with this patch, but consider myself over-voted.
Hi, On Mon, 31 Oct 2016, at 07:03, wm4 wrote: > I very strongly disagree with this patch, but consider myself > over-voted. I strongly disagree with this patch too.
Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit :
> I strongly disagree with this patch too.
We do not care about the number of people who disagree, we only care
about practical arguments.
IIRC, the last argument on the matter was mine:
http://ffmpeg.org/pipermail/ffmpeg-devel/2016-September/200317.html
If you, or anyone else opposing it, has counter-arguments, please give
them. Otherwise, the patch should go in once it is clean.
Regards,
On 10/31/2016 11:20 AM, Nicolas George wrote: > Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit : >> I strongly disagree with this patch too. > > We do not care about the number of people who disagree, we only care > about practical arguments. That's a curious thing to say when we have a voting committee to deal with polarizing subjects that went beyond practical arguments. > > IIRC, the last argument on the matter was mine: > http://ffmpeg.org/pipermail/ffmpeg-devel/2016-September/200317.html > > If you, or anyone else opposing it, has counter-arguments, please give > them. Otherwise, the patch should go in once it is clean. > > Regards, > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Le decadi 10 brumaire, an CCXXV, James Almer a écrit : > That's a curious thing to say when we have a voting committee to > deal with polarizing subjects that went beyond practical arguments. The voting committee is there for when arguments are failed: when there are significant pros and cons on each side and deciding which one is better and which one is worse is a matter of policy and taste and not a technical matter. People who try to vote instead of giving arguments are trying to abuse the power of the voting committee. Regards,
On date Monday 2016-10-31 11:27:17 -0300, James Almer encoded: > On 10/31/2016 11:20 AM, Nicolas George wrote: > > Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit : > >> I strongly disagree with this patch too. > > > > We do not care about the number of people who disagree, we only care > > about practical arguments. > > That's a curious thing to say when we have a voting committee to > deal with polarizing subjects that went beyond practical arguments. So, in practical terms, do you want me to start a formal vote about the inclusion of the format? I remember that in the last version the format is disabled by default (if people prefer I can add a dependency on an --enable-unsafe configure switch). I'm still convinced it could be useful for some testing scenarios and for data injection, but I'll commit to the decision of the voting committee in case it is rejected.
Hi, On Mon, 31 Oct 2016, at 07:20, Nicolas George wrote: > Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit : > > I strongly disagree with this patch too. > > If you, or anyone else opposing it, has counter-arguments, please give > them. Otherwise, the patch should go in once it is clean. ffprobe is not a video/audio format. It has no public specification, is made up and completely arbitrary format. It will not be used by the large majority of applications. I understand very well the need, I disagree it should go inside the (de)muxing library. Best regards
On 10/31/2016 11:30 AM, Nicolas George wrote: > Le decadi 10 brumaire, an CCXXV, James Almer a écrit : >> That's a curious thing to say when we have a voting committee to >> deal with polarizing subjects that went beyond practical arguments. > > The voting committee is there for when arguments are failed: when there > are significant pros and cons on each side and deciding which one is > better and which one is worse is a matter of policy and taste and not a > technical matter. > > People who try to vote instead of giving arguments are trying to abuse > the power of the voting committee. Afaics, the cons argumented were that this "demuxer" doesn't fit the criteria of a libavformat module. It's not demultiplexing a multimedia file. It, unless i misread it, is just reconstructing one out of a non-binary dump. This should be implemented as a separate tool using libavformat, and not as a libavformat module. See liboggz's oggz-dump tool. > > Regards, > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Le decadi 10 brumaire, an CCXXV, Stefano Sabatini a écrit : > So, in practical terms, do you want me to start a formal vote about > the inclusion of the format? NO! Until ten minutes ago, there was no actual arguments against. The technical discussion just started. Regards,
On Mon, 31 Oct 2016 15:20:15 +0100 Nicolas George <george@nsup.org> wrote: > Le decadi 10 brumaire, an CCXXV, Jean-Baptiste Kempf a écrit : > > I strongly disagree with this patch too. > > We do not care about the number of people who disagree, we only care > about practical arguments. > > IIRC, the last argument on the matter was mine: > http://ffmpeg.org/pipermail/ffmpeg-devel/2016-September/200317.html > > If you, or anyone else opposing it, has counter-arguments, please give > them. Otherwise, the patch should go in once it is clean. > I don't have the energy for such long-winded technological discussions which (predictably) lead to nothing. I just want my disagreement noted. If we have Reviewed-by etc. headers, why not Nacked-by ones?
Le decadi 10 brumaire, an CCXXV, wm4 a écrit : > I don't have the energy for such long-winded technological discussions > which (predictably) lead to nothing. I just want my disagreement noted. Well, you noted it, and I personally do not care at all without arguments. > If we have Reviewed-by etc. headers, why not Nacked-by ones? Because each commit would require approximately seven billion Nacked-by entries. Regards,
On 10/31/2016 1:27 PM, Nicolas George wrote: > Le decadi 10 brumaire, an CCXXV, wm4 a écrit : >> I don't have the energy for such long-winded technological discussions >> which (predictably) lead to nothing. I just want my disagreement noted. > > Well, you noted it, and I personally do not care at all without > arguments. > >> If we have Reviewed-by etc. headers, why not Nacked-by ones? > > Because each commit would require approximately seven billion Nacked-by > entries. So you're saying no matter how many people veto a patch it means shit and they will be committed anyway because the writer of the patch and one other person want to? One author, one positive review and seven billion Nack means the commit is approved? > > Regards, > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Le decadi 10 brumaire, an CCXXV, James Almer a écrit : > So you're saying no matter how many people veto a patch it means shit and > they will be committed anyway because the writer of the patch and one other > person want to? > > One author, one positive review and seven billion Nack means the commit is > approved? Without arguments, yes, nacks mean nothing. Regards,
On 10/31/2016 1:36 PM, Nicolas George wrote: > Le decadi 10 brumaire, an CCXXV, James Almer a écrit : >> So you're saying no matter how many people veto a patch it means shit and >> they will be committed anyway because the writer of the patch and one other >> person want to? >> >> One author, one positive review and seven billion Nack means the commit is >> approved? > > Without arguments, yes, nacks mean nothing. You not liking an argument doesn't mean the argument doesn't exist. > > Regards, > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> You not liking an argument doesn't mean the argument doesn't exist.
Indeed.
On the other hand, someone saying they do not intend to give arguments
means the argument does not exist.
That is exactly the short and long version of wm4's discourse on this
subject: "I do not have arguments, and I want the world to know it!".
Regards,
On 31/10/2016 14:15, Jean-Baptiste Kempf wrote: > Hi, > > On Mon, 31 Oct 2016, at 07:03, wm4 wrote: >> I very strongly disagree with this patch, but consider myself >> over-voted. > > > I strongly disagree with this patch too. > I am extremely against the inclusion of this patch as well. It's something which has really limited use-cases and should be a standalone tool.
Le decadi 10 brumaire, an CCXXV, Josh de Kock a écrit : > I am extremely against the inclusion of this patch as well. It's something > which has really limited use-cases and should be a standalone tool. Duplicate argument. Regards,
On 31.10.2016 09:51, Stefano Sabatini wrote: > From 7f209e27aa33e8f43444e5cfc44c68f664b69e06 Mon Sep 17 00:00:00 2001 > From: Nicolas George <george@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@gmail.com>. > > Signed-off-by: Nicolas George <george@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 > > +@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. > + 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. > + > + if (ret < 0) > + return ret; > + if (sscanf(buf, "stream_index=%d", &p.stream_index)) { > + if ((unsigned)p.stream_index >= avf->nb_streams) { > + av_log(avf, AV_LOG_ERROR, "Invalid stream number %d specified in packet number %d\n", > + p.stream_index, ffp->packet_nb); > + return AVERROR_INVALIDDATA; > + } > + has_stream_index = 1; > + } > + > +#define PARSE_TIME(name_, is_duration) \ > + sscanf(buf, #name_ "=%"SCNi64, &p.name_); \ > + has_time = sscanf(buf, #name_ "_time=%s", timebuf); \ > + if (has_time) { \ > + if (!strcmp(timebuf, "N/A")) { \ > + p.name_ = is_duration ? 0 : AV_NOPTS_VALUE; \ > + } else { \ > + ret = av_parse_time(&name_, timebuf, 1); \ > + if (ret < 0) { \ > + av_log(avf, AV_LOG_ERROR, "Invalid " #name_ " time specification '%s' for packet #%d data\n", \ > + timebuf, ffp->packet_nb); \ > + return ret; \ > + } \ > + } \ > + } \ > + > + PARSE_TIME(pts, 0); > + PARSE_TIME(dts, 0); > + PARSE_TIME(duration, 1); > + > + if (sscanf(buf, "flags=%c", &flags) >= 1) > + p.flags = flags == 'K' ? AV_PKT_FLAG_KEY : 0; > + if (!strcmp(buf, "data=")) > + if ((ret = read_data(avf, &p.size)) < 0) > + return ret; > + } > + > + if (!has_stream_index) { > + av_log(avf, AV_LOG_ERROR, "No stream index was specified for packet #%d, aborting\n", ffp->packet_nb); > + return AVERROR_INVALIDDATA; > + } > + > + if (p.size < 0 || (unsigned)p.stream_index >= avf->nb_streams) > + return SEC_NONE; > + > + p.pts = av_rescale_q(pts, AV_TIME_BASE_Q, avf->streams[p.stream_index]->time_base); > + p.dts = av_rescale_q(dts, AV_TIME_BASE_Q, avf->streams[p.stream_index]->time_base); > + p.duration = av_rescale_q(duration, AV_TIME_BASE_Q, avf->streams[p.stream_index]->time_base); > + > + if ((ret = av_new_packet(pkt, p.size)) < 0) > + return ret; > + p.data = pkt->data; > + p.buf = pkt->buf; > + *pkt = p; > + if (ffp->data.len) { > + ffp->data.len = FFMIN(ffp->data.len, pkt->size); > + memcpy(pkt->data, ffp->data.str, ffp->data.len); > + } > + return SEC_PACKET; > +} [...] > +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. Best regards, Andreas
On 4 December 2016 at 21:54, Andreas Cadhalpun < andreas.cadhalpun@googlemail.com> wrote: > On 31.10.2016 09:51, Stefano Sabatini wrote: > > From 7f209e27aa33e8f43444e5cfc44c68f664b69e06 Mon Sep 17 00:00:00 2001 > > From: Nicolas George <george@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@gmail.com>. > > > > Signed-off-by: Nicolas George <george@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'. > > What about the security reasons listed below? > > > +For security reasons this demuxer is disabled by default, should be > > +enabled though the @code{--enable-demuxer=ffprobe} configure option. > > + > > Does that mean the demuxer needs to be fuzzed or does it need to be insecure to work?
Le quartidi 14 frimaire, an CCXXV, Rostislav Pehlivanov a écrit :
> What about the security reasons listed below?
They do not exist any more than in any similar code. They were invoked
by people who did not like this patch, and the warning was added to
accommodate them.
Regards,
On 04.12.2016 23:42, Rostislav Pehlivanov wrote: > On 4 December 2016 at 21:54, Andreas Cadhalpun < > andreas.cadhalpun@googlemail.com> wrote: >> 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'. >> >> > What about the security reasons listed below? If it requires the user to explicitly add '-strict experimental', it can't be exploited in practice. Also I'm not sure there are any real security issues with this demuxer. >>> +For security reasons this demuxer is disabled by default, should be >>> +enabled though the @code{--enable-demuxer=ffprobe} configure option. >>> + >> >> > Does that mean the demuxer needs to be fuzzed or does it need to be > insecure to work? I've fuzzed it already and only found the things I mentioned. Best regards, Andreas
From 7f209e27aa33e8f43444e5cfc44c68f664b69e06 Mon Sep 17 00:00:00 2001 From: Nicolas George <george@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@gmail.com>. Signed-off-by: Nicolas George <george@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 + 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 +@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. + +This format is useful to generate streams in a textual format, easily +generated with scripting or by hand (by editing the output of +@command{ffprobe}). + +In particular, it can be also used to inject data stream generated by +scripts in the @command{ffmpeg} output, for example with the command: +@example +ffmpeg -i INPUT -i data.ffprobe -map 0:0 -map 0:1 -map 1:0 -codec:d copy OUTPUT +@end example + @section flv Adobe Flash Video Format demuxer. diff --git a/doc/ffprobe-format.texi b/doc/ffprobe-format.texi new file mode 100644 index 0000000..95ac644 --- /dev/null +++ b/doc/ffprobe-format.texi @@ -0,0 +1,121 @@ +@anchor{FFprobe demuxer format} +@chapter FFprobe demuxer format +@c man begin FFPROBE DEMUXER FORMAT + +The ffprobe demuxer format is inspired by the output generated by the +ffprobe default format. + +It consists of several sections (@samp{FORMAT}, @samp{STREAM}, +@samp{PACKET}). Each section starts with a single line in the format +@samp{[SECTION]} and ends with the corresponding line +@samp{[/SECTION]}, where @samp{SECTION} is the name of the section. + +A well-formed file consists of an initial @samp{FORMAT} section, +followed by several @samp{STREAM} sections (one per stream), and +several @samp{PACKET} sections. + +Each section contains a sequence of lines in the form +@samp{key=value}. In the case of data the key and the value must +stay on different lines. + +Unrecognized values are discarded. + +This format can be read by the @code{ffprobe} demuxer. It is an +internal format and thus should not be used for archiving purposes. + +The following sections document the fields accepted by each section. + +@section FORMAT + +@table @samp +@item nb_streams +the number of supported streams +@end table + +@section STREAM +@table @samp +@item index +the index number +@item codec_name +the codec name (the name must be an accepted FFmpeg codec name +@item time_base +Specify the time_base as @samp{NUM/DEN}, this is the time base used to +specify the timestamps in the corresponding packets +@end table + +@section PACKET + +@table @samp +@item stream_index +the stream index of the stream (must have been specified in a stream +section). If not specified the first stream is assumed. + +@item pts +the PTS expressed in the corresponding time base stream units + +@item pts_time +the PTS expressed as an absolute time, using the FFmpeg time syntax + +@item dts +the DTS expressed in the corresponding time base stream units + +@item pts_time +the DTS expressed as an absolute time, using the FFmpeg time syntax + +@item duration +the duration expressed in the corresponding time base stream units + +@item duration_time +the packet duration expressed as an absolute time, using the FFmpeg time syntax + +@item flags +If the value is equal to "K" mark the packet as a key packet + +@item data +specifies the data as a sequence of hexadecimal values (two +hexadecimal values specify a byte). The data specification starts on +the following line and can span over several lines, and must be +terminated by an empty line. Spaces within each line are discarded. + +Each line should not be longer than 4095 bytes. +@end table + +@section Examples + +An ffprobe demuxer file might look like this: +@example +[FORMAT] +nb_streams=1 +format_name=ffprobe +[/FORMAT] +[STREAM] +index=0 +codec_name=timed_id3 +time_base=1/1000000 +[/STREAM] +[PACKET] +stream_index=0 +pts_time=0 +dts_time=0 +duration=N/A +flags=K +data= +f000 0000 0167 42c0 1ed9 81b1 fefc 0440 +57ff d7d1 dfff e11a 3d7e 6cbd 0000 0001 +ce8c 4d9d 108e 25e9 fe + +[/PACKET] +[PACKET] +stream_index=0 +pts_time=1.00 +duration=N/A +flags=K +data= +f000 0000 0141 9a38 3be5 ffff ffff fffa +ebc1 3782 7d5e 21e9 ffff abf2 ea4f ed66 +0000 0001 ce8c 4d9d 108e 25e9 fe + +[/PACKET] +@end example + +@c man end FFPROBE DEMUXER FORMAT diff --git a/doc/formats.texi b/doc/formats.texi index 5ef7fad..da02e7c 100644 --- a/doc/formats.texi +++ b/doc/formats.texi @@ -251,3 +251,4 @@ The exact semantics of stream specifiers is defined by the @include muxers.texi @end ifclear @include metadata.texi +@include ffprobe-format.texi diff --git a/libavformat/Makefile b/libavformat/Makefile index 5d827d31..ede1809 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -162,6 +162,7 @@ OBJS-$(CONFIG_FFM_DEMUXER) += ffmdec.o OBJS-$(CONFIG_FFM_MUXER) += ffmenc.o OBJS-$(CONFIG_FFMETADATA_DEMUXER) += ffmetadec.o OBJS-$(CONFIG_FFMETADATA_MUXER) += ffmetaenc.o +OBJS-$(CONFIG_FFPROBE_DEMUXER) += ffprobedec.o OBJS-$(CONFIG_FIFO_MUXER) += fifo.o OBJS-$(CONFIG_FILMSTRIP_DEMUXER) += filmstripdec.o OBJS-$(CONFIG_FILMSTRIP_MUXER) += filmstripenc.o diff --git a/libavformat/allformats.c b/libavformat/allformats.c index 6a216ef..7d8d400 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -123,6 +123,7 @@ void av_register_all(void) REGISTER_MUXER (F4V, f4v); REGISTER_MUXDEMUX(FFM, ffm); REGISTER_MUXDEMUX(FFMETADATA, ffmetadata); + REGISTER_DEMUXER (FFPROBE, ffprobe); REGISTER_MUXER (FIFO, fifo); REGISTER_MUXDEMUX(FILMSTRIP, filmstrip); REGISTER_MUXDEMUX(FLAC, flac); diff --git a/libavformat/ffprobedec.c b/libavformat/ffprobedec.c new file mode 100644 index 0000000..0bc9a49 --- /dev/null +++ b/libavformat/ffprobedec.c @@ -0,0 +1,439 @@ +/* + * Copyright (c) 2013 Nicolas George + * + * 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/avassert.h" +#include "libavutil/avstring.h" +#include "libavutil/bprint.h" +#include "libavutil/parseutils.h" +#include "avformat.h" +#include "internal.h" + +enum SectionType { + SEC_NONE = 0, + SEC_FORMAT, + SEC_STREAM, + SEC_PACKET, +}; + +const char *const section_names[] = { + [SEC_NONE] = "NONE", + [SEC_FORMAT] = "FORMAT", + [SEC_STREAM] = "STREAM", + [SEC_PACKET] = "PACKET", +}; + +typedef struct { + AVClass *class; + enum SectionType section; + AVBPrint data; + int packet_nb; +} FFprobeContext; + +#define LINE_BUFFER_SIZE 4096 + +static int ffprobe_probe(AVProbeData *probe) +{ + unsigned score; + + if (!av_strstart(probe->buf, "[FORMAT]\n", NULL)) + return 0; + score = !!strstr(probe->buf, "\nnb_streams=") + + !!strstr(probe->buf, "\nnb_programs=") + + !!strstr(probe->buf, "\nformat_name=") + + !!strstr(probe->buf, "\nstart_time="); + return score >= 3 ? AVPROBE_SCORE_MAX : AVPROBE_SCORE_MAX / 2; +} + +static int ffprobe_read_close(AVFormatContext *avf) +{ + FFprobeContext *ffp = avf->priv_data; + + av_bprint_finalize(&ffp->data, NULL); + return 0; +} + +/** + * Read a section start line ("[SECTION]"). + * Update FFprobeContext.section. + * @return SectionType (>0) for success, + * SEC_NONE if no section start, + * <0 for error + */ +static int read_section_start(AVFormatContext *avf) +{ + FFprobeContext *ffp = avf->priv_data; + uint8_t buf[LINE_BUFFER_SIZE]; + const char *rest; + int i, ret; + + ret = ff_get_line(avf->pb, buf, sizeof(buf)); + if (ret == 0 && avio_feof(avf->pb)) + ret = AVERROR_EOF; + if (ret <= 0) + return ret; + if (*buf != '[') + return 0; + for (i = 1; i < FF_ARRAY_ELEMS(section_names); i++) { + if (av_strstart(buf + 1, section_names[i], &rest) && + !strcmp(rest, "]\n")) { + ffp->section = i; + return i; + } + } + return SEC_NONE; +} + +/** + * Read a line from within a section. + * @return >0 for success, 0 if end of section, <0 for error + */ +static int read_section_line(AVFormatContext *avf, uint8_t *buf, size_t size) +{ + FFprobeContext *ffp = avf->priv_data; + const char *rest; + int ret, readlen; + size_t l; + + ret = ff_get_line2(avf->pb, buf, size, &readlen); + if (ret == 0) { + av_log(avf, AV_LOG_ERROR, "Unterminated section, aborting\n"); + ret = AVERROR_INVALIDDATA; + return ret; + } + if (readlen > ret) { + av_log(avf, AV_LOG_ERROR, + "Input read line was %d bytes long, maximum supported length is %zu\n", + readlen, size-1); + return AVERROR(EINVAL); + } + if (av_strstart(buf, "[/", &rest)) { + ffp->section = 0; + return 0; + } + if ((l = strlen(buf)) > 0 && buf[l - 1] == '\n') + buf[--l] = 0; + return 1; +} + +/** + * Read hexadecimal data. + * Store it in FFprobeContext.data. + * @return >=0 for success, <0 for error + */ +static int read_data(AVFormatContext *avf, int *size) +{ + FFprobeContext *ffp = avf->priv_data; + uint8_t buf[LINE_BUFFER_SIZE], *cur; + int ret, pos, val; + + if (ffp->data.len) + return AVERROR_INVALIDDATA; + while ((ret = read_section_line(avf, buf, sizeof(buf)))) { + if (ret < 0) + return ret; + if (!buf[0]) + break; + cur = buf; + pos = 0; + cur += pos; + while (1) { + while (*cur == ' ') + cur++; + if (!*cur) + break; + if ((unsigned)(*cur - '0') >= 10 && + (unsigned)(*cur - 'a') >= 6 && + (unsigned)(*cur - 'A') >= 6) { + av_log(avf, AV_LOG_ERROR, + "Invalid character '%c' in packet number %d data\n", *cur, ffp->packet_nb); + return AVERROR_INVALIDDATA; + } + pos = 0; + if (sscanf(cur, " %02x%n", &val, &pos) < 1 || !pos) { + av_log(avf, AV_LOG_ERROR, + "Could not parse value in packet number %d data\n", ffp->packet_nb); + return AVERROR_INVALIDDATA; + } + cur += pos; + av_bprint_chars(&ffp->data, val, 1); + } + } + + if (size) + *size = ffp->data.len; + + return av_bprint_is_complete(&ffp->data) ? 0 : AVERROR(ENOMEM); +} + +#define MAX_NB_STREAMS 32 + +static int read_section_format(AVFormatContext *avf) +{ + uint8_t buf[LINE_BUFFER_SIZE]; + int ret, val; + + while ((ret = read_section_line(avf, buf, sizeof(buf)))) { + if (ret < 0) + return ret; + if (sscanf(buf, "nb_streams=%d", &val) >= 1) { + if ((unsigned)val > MAX_NB_STREAMS) { + av_log(avf, AV_LOG_ERROR, "Invalid streams number '%d', maximum allowed is %d\n", + val, MAX_NB_STREAMS); + return AVERROR_INVALIDDATA; + } + while (avf->nb_streams < val) + if (!avformat_new_stream(avf, NULL)) + return AVERROR(ENOMEM); + } + /* TODO programs */ + /* TODO start_time duration bit_rate */ + /* TODO tags */ + } + return SEC_FORMAT; +} + +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) { + 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) { + 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; + + 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]; + + if (ret < 0) + return ret; + if (sscanf(buf, "stream_index=%d", &p.stream_index)) { + if ((unsigned)p.stream_index >= avf->nb_streams) { + av_log(avf, AV_LOG_ERROR, "Invalid stream number %d specified in packet number %d\n", + p.stream_index, ffp->packet_nb); + return AVERROR_INVALIDDATA; + } + has_stream_index = 1; + } + +#define PARSE_TIME(name_, is_duration) \ + sscanf(buf, #name_ "=%"SCNi64, &p.name_); \ + has_time = sscanf(buf, #name_ "_time=%s", timebuf); \ + if (has_time) { \ + if (!strcmp(timebuf, "N/A")) { \ + p.name_ = is_duration ? 0 : AV_NOPTS_VALUE; \ + } else { \ + ret = av_parse_time(&name_, timebuf, 1); \ + if (ret < 0) { \ + av_log(avf, AV_LOG_ERROR, "Invalid " #name_ " time specification '%s' for packet #%d data\n", \ + timebuf, ffp->packet_nb); \ + return ret; \ + } \ + } \ + } \ + + PARSE_TIME(pts, 0); + PARSE_TIME(dts, 0); + PARSE_TIME(duration, 1); + + if (sscanf(buf, "flags=%c", &flags) >= 1) + p.flags = flags == 'K' ? AV_PKT_FLAG_KEY : 0; + if (!strcmp(buf, "data=")) + if ((ret = read_data(avf, &p.size)) < 0) + return ret; + } + + if (!has_stream_index) { + av_log(avf, AV_LOG_ERROR, "No stream index was specified for packet #%d, aborting\n", ffp->packet_nb); + return AVERROR_INVALIDDATA; + } + + if (p.size < 0 || (unsigned)p.stream_index >= avf->nb_streams) + return SEC_NONE; + + p.pts = av_rescale_q(pts, AV_TIME_BASE_Q, avf->streams[p.stream_index]->time_base); + p.dts = av_rescale_q(dts, AV_TIME_BASE_Q, avf->streams[p.stream_index]->time_base); + p.duration = av_rescale_q(duration, AV_TIME_BASE_Q, avf->streams[p.stream_index]->time_base); + + if ((ret = av_new_packet(pkt, p.size)) < 0) + return ret; + p.data = pkt->data; + p.buf = pkt->buf; + *pkt = p; + if (ffp->data.len) { + ffp->data.len = FFMIN(ffp->data.len, pkt->size); + memcpy(pkt->data, ffp->data.str, ffp->data.len); + } + return SEC_PACKET; +} + +static int read_section(AVFormatContext *avf, AVPacket *pkt) +{ + FFprobeContext *ffp = avf->priv_data; + int ret, section; + + while (!ffp->section) + if ((ret = read_section_start(avf)) < 0) + return ret; + switch (section = ffp->section) { + case SEC_FORMAT: + return read_section_format(avf); + case SEC_STREAM: + return read_section_stream(avf); + case SEC_PACKET: + ret = read_section_packet(avf, pkt); + ffp->packet_nb++; + return ret; + default: + av_assert0(!"reached"); + return AVERROR_BUG; + } +} + +static int ffprobe_read_header(AVFormatContext *avf) +{ + FFprobeContext *ffp = avf->priv_data; + int ret; + int nb_streams = 0; + + av_bprint_init(&ffp->data, 0, AV_BPRINT_SIZE_UNLIMITED); + if ((ret = read_section_start(avf)) < 0) + return ret; + if (ret != SEC_FORMAT) { + av_log(avf, AV_LOG_INFO, "Using noheader mode\n"); + avf->ctx_flags |= AVFMTCTX_NOHEADER; + return 0; + } + if ((ret = read_section_format(avf)) < 0) + return ret; + + /* read stream information */ + while (1) { + ret = read_section_start(avf); + if (ret != SEC_STREAM) + break; + if ((ret = read_section_stream(avf)) < 0) + return ret; + nb_streams++; + } + + if (nb_streams != avf->nb_streams) { + av_log(avf, AV_LOG_ERROR, "Number of declared streams is %d, but only %d streams were specified in STREAM sections\n", + avf->nb_streams, nb_streams); + return AVERROR_INVALIDDATA; + } + + return 0; +} + +static int ffprobe_read_packet(AVFormatContext *avf, AVPacket *pkt) +{ + int ret; + + while (1) { + if ((ret = read_section(avf, pkt)) < 0) + return ret; + if (ret == SEC_PACKET) + return 0; + } +} + +static const AVClass ffprobe_class = { + .class_name = "ffprobe demuxer", + .item_name = av_default_item_name, + .version = LIBAVUTIL_VERSION_INT, +}; + +AVInputFormat ff_ffprobe_demuxer = { + .name = "ffprobe", + .long_name = NULL_IF_CONFIG_SMALL("FFprobe alike output"), + .priv_data_size = sizeof(FFprobeContext), + .read_probe = ffprobe_probe, + .read_header = ffprobe_read_header, + .read_packet = ffprobe_read_packet, + .read_close = ffprobe_read_close, + .priv_class = &ffprobe_class, +}; -- 1.9.1