Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2017-01-02 Thread Stefano Sabatini
On date Tuesday 2016-12-13 00:16:24 +0100, Andreas Cadhalpun encoded:
> On 10.12.2016 17:55, Stefano Sabatini wrote:
> > From ebc34da37648a07f25da94a1662c278c13ca7383 Mon Sep 17 00:00:00 2001
> > From: Nicolas George 
> > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > Subject: [PATCH] lavf: add ffprobe demuxer
> > 
> > With several modifications and documentation by Stefano Sabatini
> > .
> > 
> > Signed-off-by: Nicolas George 
> > ---
> >  doc/demuxers.texi|  19 ++
> >  doc/ffprobe-format.texi  | 121 +
> >  doc/formats.texi |   1 +
> >  libavformat/Makefile |   1 +
> >  libavformat/allformats.c |   1 +
> >  libavformat/ffprobedec.c | 452 
> > +++
> >  6 files changed, 595 insertions(+)
> >  create mode 100644 doc/ffprobe-format.texi
> >  create mode 100644 libavformat/ffprobedec.c
> > 
> [...]
> > diff --git a/libavformat/ffprobedec.c b/libavformat/ffprobedec.c
> > new file mode 100644
> > index 000..f5d5ed7
> > --- /dev/null
> > +++ b/libavformat/ffprobedec.c
> [...]
> > +if (av_strstart(buf, "codec_name=", )) {
> > +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);
> 

> This log message is missing a newline at the end.
> 
> Other than that this only needs adding the format to doc/general.texi,
> a minor version bump and a changelog entry.

Updated.
-- 
FFmpeg = Friendly & Forgiving Mournful Proud Evil Guru
>From 37d6308014eacba11a87b0df86f95b94814bd663 Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Sat, 11 Jan 2014 19:42:41 +0100
Subject: [PATCH] lavf: add ffprobe demuxer

With several modifications and documentation by Stefano Sabatini
.

TODO: bump libavformat minor

Signed-off-by: Nicolas George 
---
 Changelog|   2 +
 doc/demuxers.texi|  19 ++
 doc/ffprobe-format.texi  | 121 +
 doc/formats.texi |   1 +
 libavformat/Makefile |   1 +
 libavformat/allformats.c |   1 +
 libavformat/ffprobedec.c | 452 +++
 7 files changed, 597 insertions(+)
 create mode 100644 doc/ffprobe-format.texi
 create mode 100644 libavformat/ffprobedec.c

diff --git a/Changelog b/Changelog
index aff9ab0..6d05963 100644
--- a/Changelog
+++ b/Changelog
@@ -12,6 +12,8 @@ version :
 - 16.8 floating point pcm decoder
 - 24.0 floating point pcm decoder
 - Apple Pixlet decoder
+- ffprobe demuxer
+
 
 version 3.2:
 - libopenmpt demuxer
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index c12c07e..9eb7f20 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -260,6 +260,25 @@ ffmpeg -f live_flv -i rtmp:///anything/key 
 Allocate the streams according to the onMetaData array content.
 @end table
 
+@section ffprobe
+
+ffprobe internal demuxer. It is able to demux files in the format
+specified in the @ref{FFprobe demuxer format} chapter.
+
+This demuxer is experimental, use it with the libavformat
+@option{strict_std_compliance} option set to @code{"experimental"}
+(use @code{-strict experimental} with the commandline tools).
+
+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 -f_strict experimental -map 0:0 -map 0:1 -map 1:0 -codec:d copy OUTPUT
+@end example
+
 @section gif
 
 Animated GIF demuxer.
diff --git a/doc/ffprobe-format.texi b/doc/ffprobe-format.texi
new file mode 100644
index 000..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

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-12 Thread Andreas Cadhalpun
On 10.12.2016 17:55, Stefano Sabatini wrote:
> From ebc34da37648a07f25da94a1662c278c13ca7383 Mon Sep 17 00:00:00 2001
> From: Nicolas George 
> Date: Sat, 11 Jan 2014 19:42:41 +0100
> Subject: [PATCH] lavf: add ffprobe demuxer
> 
> With several modifications and documentation by Stefano Sabatini
> .
> 
> Signed-off-by: Nicolas George 
> ---
>  doc/demuxers.texi|  19 ++
>  doc/ffprobe-format.texi  | 121 +
>  doc/formats.texi |   1 +
>  libavformat/Makefile |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/ffprobedec.c | 452 
> +++
>  6 files changed, 595 insertions(+)
>  create mode 100644 doc/ffprobe-format.texi
>  create mode 100644 libavformat/ffprobedec.c
> 
[...]
> diff --git a/libavformat/ffprobedec.c b/libavformat/ffprobedec.c
> new file mode 100644
> index 000..f5d5ed7
> --- /dev/null
> +++ b/libavformat/ffprobedec.c
[...]
> +if (av_strstart(buf, "codec_name=", )) {
> +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);

This log message is missing a newline at the end.

Other than that this only needs adding the format to doc/general.texi,
a minor version bump and a changelog entry.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-10 Thread Stefano Sabatini
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 
> > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > Subject: [PATCH] lavf: add ffprobe demuxer
> > 
> > With several modifications and documentation by Stefano Sabatini
> > .
> > 
> > Signed-off-by: Nicolas George 
> > ---
> >  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 000..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(>data);
> > +while ((ret = read_section_line(avf, buf, sizeof(buf {
> > +if (ret < 0)
> > +return ret;
> > +if (!st) {
> > +if (sscanf(buf, "index=%d", ) >= 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=", )) {
> > +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(>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", , ) >= 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",
> 

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-09 Thread Andreas Cadhalpun
On 09.12.2016 15:55, Stefano Sabatini wrote:
> On date Friday 2016-11-25 11:58:42 +0100, Nicolas George encoded:
>> Le quintidi 5 frimaire, an CCXXV, Andreas Cadhalpun a écrit :
>>> I think a better idea would be to require '-strict experimental',
>>> as code disabled by default does neither get build- nor FATE-tested
>>> much.
>>
>> That is an excellent idea!
> 
> -strict is for codecs, there is no such thing at the moment for
> muxers/demuxers (so it should be implemented for muxers/demuxers).

It works fine for the mp4 muxer, just try it yourself:
$ ffmpeg -f lavfi -i testsrc=d=1 -c libvpx-vp9 -f mp4 /dev/null -y
$ ffmpeg -f lavfi -i testsrc=d=1 -c libvpx-vp9 -f mp4 -strict experimental 
/dev/null -y

I even showed how it can be implemented in this case [1]:
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;
}

Best regards,
Andreas


1: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-December/203901.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-09 Thread Hendrik Leppkes
On Fri, Dec 9, 2016 at 3:59 PM, Nicolas George  wrote:
> Le nonidi 19 frimaire, an CCXXV, Stefano Sabatini a écrit :
>> -strict is for codecs, there is no such thing at the moment for
>> muxers/demuxers (so it should be implemented for muxers/demuxers).
>
> AVFormatContext has strict_std_compliance too.
>

-f_strict through the ffmpeg CLI, FWIW, to avoid clashes with the
codec-level -strict.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-09 Thread Nicolas George
Le nonidi 19 frimaire, an CCXXV, Stefano Sabatini a écrit :
> -strict is for codecs, there is no such thing at the moment for
> muxers/demuxers (so it should be implemented for muxers/demuxers).

AVFormatContext has strict_std_compliance too.

Regards,

-- 
  Nicolas George
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-09 Thread Stefano Sabatini
On date Friday 2016-11-25 11:58:42 +0100, Nicolas George encoded:
> Le quintidi 5 frimaire, an CCXXV, Andreas Cadhalpun a écrit :
> > I think a better idea would be to require '-strict experimental',
> > as code disabled by default does neither get build- nor FATE-tested
> > much.
> 
> That is an excellent idea!

-strict is for codecs, there is no such thing at the moment for
muxers/demuxers (so it should be implemented for muxers/demuxers).
-- 
FFmpeg = Faithless and Free Magic Patchable Evanescent Guide
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-04 Thread Andreas Cadhalpun
On 04.12.2016 23:42, Rostislav Pehlivanov wrote:
> On 4 December 2016 at 21:54, Andreas Cadhalpun <
> andreas.cadhal...@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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-04 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-04 Thread Rostislav Pehlivanov
On 4 December 2016 at 21:54, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:

> On 31.10.2016 09:51, Stefano Sabatini wrote:
> > From 7f209e27aa33e8f43444e5cfc44c68f664b69e06 Mon Sep 17 00:00:00 2001
> > From: Nicolas George 
> > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > Subject: [PATCH] lavf: add ffprobe demuxer
> >
> > With several modifications and documentation by Stefano Sabatini
> > .
> >
> > Signed-off-by: Nicolas George 
> > ---
> >  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?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-12-04 Thread Andreas Cadhalpun
On 31.10.2016 09:51, Stefano Sabatini wrote:
> From 7f209e27aa33e8f43444e5cfc44c68f664b69e06 Mon Sep 17 00:00:00 2001
> From: Nicolas George 
> Date: Sat, 11 Jan 2014 19:42:41 +0100
> Subject: [PATCH] lavf: add ffprobe demuxer
> 
> With several modifications and documentation by Stefano Sabatini
> .
> 
> Signed-off-by: Nicolas George 
> ---
>  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 000..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(>data);
> +while ((ret = read_section_line(avf, buf, sizeof(buf {
> +if (ret < 0)
> +return ret;
> +if (!st) {
> +if (sscanf(buf, "index=%d", ) >= 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=", )) {
> +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(>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", , ) >= 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 

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-11-25 Thread Nicolas George
Le quintidi 5 frimaire, an CCXXV, Andreas Cadhalpun a écrit :
> I think a better idea would be to require '-strict experimental',
> as code disabled by default does neither get build- nor FATE-tested
> much.

That is an excellent idea!

Regards,

-- 
  Nicolas George
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-11-24 Thread Andreas Cadhalpun
On 24.11.2016 12:36, Stefano Sabatini wrote:
> I think the most important controversy points revolve around these facts:
> 
> * developers do not want to add support for an internal only format,
>   which adds to the maintainance burden with no clear benefit, since
>   there are no clear use cases outside the scope of internal debugging
>   and specific tasks. Let's call this argument "maintainance burden".
> 
> * the second argument is about the fact that such new format is not a
>   "standard" format, so it's not really interoperable. Let's call this
>   argument "non interoperability".
> 
> The two facts combine so that some developers claim that there is an
> added maintainance burden for a non interoperable format.
> 
> I'm not able to read more technical/design arguments. I suppose this
> cannot really be discussed more, since one hardly accepts more burden
> if he has no apparent benefits from the burden, so I'd expect that
> only the developers who see the gain will agree with adding the new
> format.

I think a significant reason for people objecting to this format is
the experience with the ffmpeg-specific ffm format, which unfortunately
wasn't very well designed and thus caused security issues as well as
incompatibilities between different versions of ffmpeg.

However, the ffprobe demuxer code looks a lot better in both regards,
because it neither uses ffmpeg internal stuff like codec_id numbers
(which breaks compatibility, when these change), nor allows setting
arbitrary AVOptions from the file (which can cause security issues).

By the way, the gain I see in this demuxer is that it would be rather
useful for fuzz-testing, as it supports all codecs.

> We can mitigate the burden effect by reducing the impact of the
> revelant code (for example making it disabled by default),

I think a better idea would be to require '-strict experimental',
as code disabled by default does neither get build- nor FATE-tested
much.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-11-24 Thread Stefano Sabatini
On date Tuesday 2016-11-01 15:49:12 +0100, Michael Niedermayer encoded:
> On Tue, Nov 01, 2016 at 02:45:00AM -0300, James Almer wrote:
[...]
> > Libavformat is not a dumping ground for code molded by a single person's
> > specific needs, and it is not a library meant to hold your or anyone's
> > debug tools.
> 
> playing devils advocate, why not?
> 
> If a group of developers needs some code to debug and maintain a part
> of FFmpeg it does seem logic to me to dump that in the codebase if it
> doesnt affect anyone else except psycological.
> 
> we can play this thought experiment even more evil.
> Consider a fork
> one where developers dump the code they need to maintain and debug
> in the code base (strictly limited to what does not affect others
> technically)
> and one fork where people cannot do this
> 
> Who of the 2 is more "fit" for survival in a evolutionary / darwinian
> sense ?
> A project where people can just debug and maintain their code or a
> project where its more cumbersome in some way (seperate repo, seperate
> tool that only works in a subset of cases, ...)?

This is a great reply, and I fully agree with this point of view.
 
> I dont want to be offensive but to me one side of this argument is
> completely absurd
> maybe iam an idiot and just dont see it, but i really dont understand
> why we would want to make it harder for ourselfs to maintain and debug
> things.
> 
> Now after writing this, theres somethig i realize
> Is the issue people have with the text/ffprobe demuxer that it is not
> a standarized format ?
> Would writing a formal specification resolve this whole disagreement ?

I think the most important controversy points revolve around these facts:

* developers do not want to add support for an internal only format,
  which adds to the maintainance burden with no clear benefit, since
  there are no clear use cases outside the scope of internal debugging
  and specific tasks. Let's call this argument "maintainance burden".

* the second argument is about the fact that such new format is not a
  "standard" format, so it's not really interoperable. Let's call this
  argument "non interoperability".

The two facts combine so that some developers claim that there is an
added maintainance burden for a non interoperable format.

I'm not able to read more technical/design arguments. I suppose this
cannot really be discussed more, since one hardly accepts more burden
if he has no apparent benefits from the burden, so I'd expect that
only the developers who see the gain will agree with adding the new
format.

We can mitigate the burden effect by reducing the impact of the
revelant code (for example making it disabled by default), and
clarifying that it must be used only for internal development or for
introducing features which would be otherwise more difficult to
implement (for example scripting media generation/processing), and
shouldn't be used for archiving purposes.

These are the only counter-arguments I can offer and implement.

> I mean a real actual quality spec from which one could write both
> a muxer and demuxer that can interoperate

I don't think that extending the specification would be really useful
since I don't want to make the format interoperable *at this point*
(since we may not be aware of faults in the format itself) but I plan
to use it only internally, so I'm not very well disposed to pay the
additional time and effort it requires for something which I won't use.

The other more generic argument was explained by Michael, the added
functionality is a bonus from an evolutionary point of view since it
fosters contributions. Said in different words, I remind you the old
saying that "perfect is the enemy of the good", so I don't consider
this new format perfect or fit for all the tasks, but good enough for
what I need to do.

That said, I have really no time and energy to prolong this discussion
ad infinitum, so I'd call for a formal vote (in the case this is not
accepted, I could maintain it externally although I don't think this
is a good idea).
-- 
FFmpeg = Forgiving and Forgiving Most Pitiful Enlightened Gadget
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-11-03 Thread Nicolas George
Le primidi 11 brumaire, an CCXXV, James Almer a écrit :
> No. You insist this is about technical merits, and i already mentioned this
> is not about that.

What else is there?

> I'll repeat what i said. This is not a technical discussion. This is a
> design issue.

I really do not understand you. A design issue IS a technical issue.

For the rest, see others' replies.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-11-01 Thread Michael Niedermayer
On Tue, Nov 01, 2016 at 02:45:00AM -0300, James Almer wrote:
> On 10/31/2016 3:30 PM, Nicolas George wrote:
> > Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
[...]
> >
> > * What benefits do you see for the separate tool approach?
> > 
> > * Can you negate any of these drawbacks?
> 
> I'll repeat what i said. This is not a technical discussion. This is a
> design issue. For your very specific debugging needs and scenario, you
> devised a way to dump media streams into a text file and declared said 
> dumps a multimedia "format" so they may qualify for a demuxer.

not being nicolas, and not having desiged this and I admit fully
not understanding the design issue.
Most generically

Something that takes a linear stream of bytes (possibly finite possibly
not) (possibly seekable, possibly not) and spits out ordered
byte sequences (packets) organized by streams, maybe with timestamps
metadata, chapters, ...
Is something I would call a demuxer
Or rather thats how i would define a demuxer.

something taking a linear text stream (ffprobe formating) that spits
out ordered  byte sequences (packets) organized by streams, maybe with
timestamps metadata, chapters, ...

Is by that definition also a demuxer.

While for example a video capture device would NOT be a demuxer, it
doesnt have a linear byte stream as input, it in fact doesnt have
an input from the point of view of the software.

I do understand the issue people had with demuxers being used as video
capture API but i dont understand it here for a ascii/utf8 instead of
a standarized with spec binary input.



> Libavformat is not a dumping ground for code molded by a single person's
> specific needs, and it is not a library meant to hold your or anyone's
> debug tools.

playing devils advocate, why not?

If a group of developers needs some code to debug and maintain a part
of FFmpeg it does seem logic to me to dump that in the codebase if it
doesnt affect anyone else except psycological.

we can play this thought experiment even more evil.
Consider a fork
one where developers dump the code they need to maintain and debug
in the code base (strictly limited to what does not affect others
technically)
and one fork where people cannot do this

Who of the 2 is more "fit" for survival in a evolutionary / darwinian
sense ?
A project where people can just debug and maintain their code or a
project where its more cumbersome in some way (seperate repo, seperate
tool that only works in a subset of cases, ...)?

I dont want to be offensive but to me one side of this argument is
completely absurd
maybe iam an idiot and just dont see it, but i really dont understand
why we would want to make it harder for ourselfs to maintain and debug
things.

Now after writing this, theres somethig i realize
Is the issue people have with the text/ffprobe demuxer that it is not
a standarized format ?
Would writing a formal specification resolve this whole disagreement ?

I mean a real actual quality spec from which one could write both
a muxer and demuxer that can interoperate


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-11-01 Thread Stefano Sabatini
On date Tuesday 2016-11-01 02:45:00 -0300, James Almer encoded:
> On 10/31/2016 3:30 PM, Nicolas George wrote:
> > Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
[...]
> > * What benefits do you see for the separate tool approach?
> > 
> > * Can you negate any of these drawbacks?
> 
> I'll repeat what i said. This is not a technical discussion. This is a
> design issue. For your very specific debugging needs and scenario, you
> devised a way to dump media streams into a text file and declared said 
> dumps a multimedia "format" so they may qualify for a demuxer.
> Libavformat is not a dumping ground for code molded by a single person's
> specific needs, and it is not a library meant to hold your or anyone's
> debug tools.
> 
> At least three developers have stated being against the implementation
> of such an arbitrary custom format, and as much as you want to cover your
> ears, declare it's all "duplicate arguments" in order to downplay and
> discard their opinions, and decree that no matter how many people are
> against something from a design PoV the only thing that matters are
> technical arguments limited to the realm of how to implement the solution
> for the very specific scenario you're presenting, if more people are
> against it than in favor then it should not go in, or a vote should be
> called.
> 

> And why not just keep it in a separate repo for that matter? Why does it
> need to be committed to the main repo?

It depends. If implemented as a libavformat component, it's impossible
to publish it in a separate repository. If implemented as a standalone
tool that would be possible, but still would require to duplicate the
FFmpeg infrastructure (mostly the configure/build system).

For my first use case (injecting data streams) it would be much easier
to write a custom tool. I was asked to integrate this into FFmpeg to
avoid the conversion step (custom format => standard format), and
while doing this I was suggested to use the more general ffprobe
format, which although not a real format was something which was
already used internally by the project.

Talking about conversion tools (say it ffprobe-dump and
ffprobe-undump), I wonder how they would work. I expect something as:

ffprobe-dump IN > OUT.ffprobe

(this could also be a modified version of ffprobe, probably with some
custom options).

Now you need to import the file (possibly after you edit it). The
problem here is that you need to convert it to another internally
supported format, using libavformat. That's when it becomes
inconvenient, because you basically need to use the library to convert
the format (see the arguments by Nicolas and Michael).

> You talk about maintenance, yet keeping it in a separate repository would
> mean as much maintenance for you as it would in the main repository.

I see the validity of the arguments for not integrating this into
FFmpeg, yet (sorry for repeating), there are also valid arguments in
favor of the integration, that's why I proposed a vote since I think
that technical arguments cannot resolve the decision alone, since it
also involves design and policy (which depend on how each every
developer conceives the project, which is somehow subjective).
-- 
FFmpeg = Foolish & Faithless Mortal Proud Ermetic Gigant
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 3:30 PM, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
>> It's an scenario that could happen, like it or not. I'd rather not open the
>> doors for it.
> 
> "Opening doors" sounds like a different wording for the slippery slope
> fallacy.

Would you prefer "setting a precedent", then?

> 
>> "why this format and not this other" is coincidentally the argument that
>> will be used after this thing gets committed to justify adding others.
> 
> And the answer in this case is very easy: if the format is more useful
> than harmful, it goes in.

No. You insist this is about technical merits, and i already mentioned this
is not about that.

> 
>> If something is to create these files, it should be ffprobe itself and
>> not some unrelated muxer that's going to duplicate a lot of code.
> 
> I agree, that would be best.
> 
>> It might, hence why i suggested this being implemented as a standalone tool
>> and not that you should drop the whole thing.
> 
> Ok, thanks.
> 
>> You presented an scenario and use case to dump media streams into a text
>> file to easily alter them, analyze them and reconstruct them. You then
>> declared said dumps to be an actual format so they could qualify for an
>> avformat module that could digest them.
>>
>> There are no technical drawbacks for this demuxer.
> 
> Again, thanks. Therefore, let us discuss the rest:
> 
>>   This is a concept and
>> design issue. It's a very specific need so far a single person or two
>> that's being forced into the libraries because it's the easiest way to
>> implement it.
>>
>> This should be written as a standalone tool that uses lavf API to rebuild
>> the custom xml-like dump with the hex formatted packets as created by
>> ffprobe. Basically, write ffmpeg's own, more powerful and versatile
>> version of oggz-dump that uses lavf instead of libogg, meaning the dumps
>> can be created and rebuilt from and into any lavf supported format.
> 
> Ok. But compared with the demuxer approach, I see several drawbacks
> (some of which I have already posted):
> 
> - More code, therefore more maintenance burden.
> 
> - When used for debugging (this is the main purpose of this feature),
>   more code path, and therefore more room to trigger unrelated bugs that
>   will waste time.
> 
> - Subject to the limitations of the intermediate format.
> 
>   This one is especially important: imagine I am trying to investigate a
>   bug that happens with files with inconsistent timestamps. I can edit
>   the dump to make the timestamps inconsistent. And then... I try to
>   rebuild a file based on the dump, and lavf (used by the undump tool)
>   forbids me to mux inconsistent timestamps. Game over.
> 
>   The whole point of this demuxer is to be able to build slightly broken
>   input, because they are the cases that need debugging the most.
>   Unfortunately, if you convert to an actual format, some of the
>   brokenness will be fixed on the fly or flat out impossible.
> 
> - More steps for the person who is doing the debugging. Considering some
>   debugging sessions will require hundreds of rounds, the difference
>   will add up. Not just the generation step, but also the half hour lost
>   because the developer forgot to convert after a change in the text
>   file.
> 
> Now, your move:

This is not a competition, Nicolas, and this is not a technical merits
discussion. Nobody doubts a lavf module would be much more versatile and
powerful than a standalone tool.

> 
> * What benefits do you see for the separate tool approach?
> 
> * Can you negate any of these drawbacks?

I'll repeat what i said. This is not a technical discussion. This is a
design issue. For your very specific debugging needs and scenario, you
devised a way to dump media streams into a text file and declared said 
dumps a multimedia "format" so they may qualify for a demuxer.
Libavformat is not a dumping ground for code molded by a single person's
specific needs, and it is not a library meant to hold your or anyone's
debug tools.

At least three developers have stated being against the implementation
of such an arbitrary custom format, and as much as you want to cover your
ears, declare it's all "duplicate arguments" in order to downplay and
discard their opinions, and decree that no matter how many people are
against something from a design PoV the only thing that matters are
technical arguments limited to the realm of how to implement the solution
for the very specific scenario you're presenting, if more people are
against it than in favor then it should not go in, or a vote should be
called.

And why not just keep it in a separate repo for that matter? Why does it
need to be committed to the main repo?
You talk about maintenance, yet keeping it in a separate repository would
mean as much maintenance for you as it would in the main repository.

> 
> Regards,
> 
> 
> 
> ___

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Michael Niedermayer
On Mon, Oct 31, 2016 at 07:22:58PM +0100, Michael Niedermayer wrote:
> On Mon, Oct 31, 2016 at 02:41:05PM -0300, James Almer wrote:
> > On 10/31/2016 1:54 PM, Nicolas George wrote:
> > > Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> [...]
> > > 
> > >> Which so happens to be what every libav* user has to do for their 
> > >> projects.
> > >> Write a program using the libraries' public API to read and write files.
> > >>
> > >> We have plenty of times refused code people wanted to dump into 
> > >> libavformat
> > >> and libavcodec because it was the easiest way for them to get what they
> > >> needed done. There was this absolutely insane youtube-dl "demuxer" that 
> > >> still
> > >> gives me the chills.
> > >> We have also plenty of times let things go in, even with several 
> > >> developers
> > >> against it. Lets try to not do it again.
> > > 
> > > This argument mixes a whole lot of different cases together. Each case
> > > has its own specifics that should be used to judge whether it should
> > > have been accepted or not.
> > > 
> > > Once again: usefulness and drawbacks.
> > > 
> > > Having one demuxer of this kind, as powerful as possible, is useful. Do
> > > you not agree with that?
> > 
> > It might, hence why i suggested this being implemented as a standalone tool
> > and not that you should drop the whole thing.
> > 
> > > 
> > > (Having several of them would be less useful.)
> > > 
> > > What are the drawbacks you see?
> > 
> > You presented an scenario and use case to dump media streams into a text
> > file to easily alter them, analyze them and reconstruct them. You then
> > declared said dumps to be an actual format so they could qualify for an
> > avformat module that could digest them.
> > 
> > There are no technical drawbacks for this demuxer. This is a concept and
> > design issue. It's a very specific need so far a single person or two
> > that's being forced into the libraries because it's the easiest way to
> > implement it.
> > 
> > This should be written as a standalone tool that uses lavf API to rebuild
> > the custom xml-like dump with the hex formatted packets as created by
> > ffprobe. Basically, write ffmpeg's own, more powerful and versatile
> > version of oggz-dump that uses lavf instead of libogg, meaning the dumps
> > can be created and rebuilt from and into any lavf supported format.
> 
> A standalone tool would make some usecases more cumbersome
> 
> for example if you are debuging a video player which uses libavformat
> and libavcodec.
> You would dump a testcase into the text format by whatever means.
> you now could edit this text file by any text editor or grep/sed tools
> and then test if a problem still occurs to "bisect" down to the spot
> in the file causing it to create a small testcase or to debug and
> fix it.
> 
> The difference is with a demuxer you can directly play the text file
> seek in it possibly do any test you need to.
> with a standalone tool you first need to convert the text file back
> into a standard media file.
> The additional step may be slow if the file is large,
> It may be difficult or impossibly to do if the case one wants to test
> is not supported by the syntax of any binary format or one does not
> know which binary format supports the combination of features one wants
> to test. 

Another case where a standalone tool would have a problem is the
case where we dont have a muxer.

To show a specific example:
Consider a bethsoftvid file, converting it to "text" and editing it
one is stuck as we have no muxer for bethsoftvid and the video codec
in it is not supported by any other muxer



[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> It's an scenario that could happen, like it or not. I'd rather not open the
> doors for it.

"Opening doors" sounds like a different wording for the slippery slope
fallacy.

> "why this format and not this other" is coincidentally the argument that
> will be used after this thing gets committed to justify adding others.

And the answer in this case is very easy: if the format is more useful
than harmful, it goes in.

> If something is to create these files, it should be ffprobe itself and
> not some unrelated muxer that's going to duplicate a lot of code.

I agree, that would be best.

> It might, hence why i suggested this being implemented as a standalone tool
> and not that you should drop the whole thing.

Ok, thanks.

> You presented an scenario and use case to dump media streams into a text
> file to easily alter them, analyze them and reconstruct them. You then
> declared said dumps to be an actual format so they could qualify for an
> avformat module that could digest them.
> 
> There are no technical drawbacks for this demuxer.

Again, thanks. Therefore, let us discuss the rest:

>This is a concept and
> design issue. It's a very specific need so far a single person or two
> that's being forced into the libraries because it's the easiest way to
> implement it.
> 
> This should be written as a standalone tool that uses lavf API to rebuild
> the custom xml-like dump with the hex formatted packets as created by
> ffprobe. Basically, write ffmpeg's own, more powerful and versatile
> version of oggz-dump that uses lavf instead of libogg, meaning the dumps
> can be created and rebuilt from and into any lavf supported format.

Ok. But compared with the demuxer approach, I see several drawbacks
(some of which I have already posted):

- More code, therefore more maintenance burden.

- When used for debugging (this is the main purpose of this feature),
  more code path, and therefore more room to trigger unrelated bugs that
  will waste time.

- Subject to the limitations of the intermediate format.

  This one is especially important: imagine I am trying to investigate a
  bug that happens with files with inconsistent timestamps. I can edit
  the dump to make the timestamps inconsistent. And then... I try to
  rebuild a file based on the dump, and lavf (used by the undump tool)
  forbids me to mux inconsistent timestamps. Game over.

  The whole point of this demuxer is to be able to build slightly broken
  input, because they are the cases that need debugging the most.
  Unfortunately, if you convert to an actual format, some of the
  brokenness will be fixed on the fly or flat out impossible.

- More steps for the person who is doing the debugging. Considering some
  debugging sessions will require hundreds of rounds, the difference
  will add up. Not just the generation step, but also the half hour lost
  because the developer forgot to convert after a change in the text
  file.

Now, your move:

* What benefits do you see for the separate tool approach?

* Can you negate any of these drawbacks?

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Michael Niedermayer
On Mon, Oct 31, 2016 at 02:41:05PM -0300, James Almer wrote:
> On 10/31/2016 1:54 PM, Nicolas George wrote:
> > Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
[...]
> > 
> >> Which so happens to be what every libav* user has to do for their projects.
> >> Write a program using the libraries' public API to read and write files.
> >>
> >> We have plenty of times refused code people wanted to dump into libavformat
> >> and libavcodec because it was the easiest way for them to get what they
> >> needed done. There was this absolutely insane youtube-dl "demuxer" that 
> >> still
> >> gives me the chills.
> >> We have also plenty of times let things go in, even with several developers
> >> against it. Lets try to not do it again.
> > 
> > This argument mixes a whole lot of different cases together. Each case
> > has its own specifics that should be used to judge whether it should
> > have been accepted or not.
> > 
> > Once again: usefulness and drawbacks.
> > 
> > Having one demuxer of this kind, as powerful as possible, is useful. Do
> > you not agree with that?
> 
> It might, hence why i suggested this being implemented as a standalone tool
> and not that you should drop the whole thing.
> 
> > 
> > (Having several of them would be less useful.)
> > 
> > What are the drawbacks you see?
> 
> You presented an scenario and use case to dump media streams into a text
> file to easily alter them, analyze them and reconstruct them. You then
> declared said dumps to be an actual format so they could qualify for an
> avformat module that could digest them.
> 
> There are no technical drawbacks for this demuxer. This is a concept and
> design issue. It's a very specific need so far a single person or two
> that's being forced into the libraries because it's the easiest way to
> implement it.
> 
> This should be written as a standalone tool that uses lavf API to rebuild
> the custom xml-like dump with the hex formatted packets as created by
> ffprobe. Basically, write ffmpeg's own, more powerful and versatile
> version of oggz-dump that uses lavf instead of libogg, meaning the dumps
> can be created and rebuilt from and into any lavf supported format.

A standalone tool would make some usecases more cumbersome

for example if you are debuging a video player which uses libavformat
and libavcodec.
You would dump a testcase into the text format by whatever means.
you now could edit this text file by any text editor or grep/sed tools
and then test if a problem still occurs to "bisect" down to the spot
in the file causing it to create a small testcase or to debug and
fix it.

The difference is with a demuxer you can directly play the text file
seek in it possibly do any test you need to.
with a standalone tool you first need to convert the text file back
into a standard media file.
The additional step may be slow if the file is large,
It may be difficult or impossibly to do if the case one wants to test
is not supported by the syntax of any binary format or one does not
know which binary format supports the combination of features one wants
to test. 

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 1:54 PM, Nicolas George wrote:
> Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
>> Someone will come up with an hexdump demuxer next and call hexdump output
>> a multimedia file, if we follow your views. Or an XML parser for a similarly
>> arbitrary custom format they happened to devise. Or maybe oggz-dump's output.
>>
>> While for the former we can tell them to go away, we wouldn't have many
>> arguments against the latter two if this patch goes in. Because telling them
>> we don't want their custom format after this one was committed would be
>> hypocritical, biased, and a decision as arbitrary as the actual hypothetical
>> format.
> 
> If these formats are useful, they should be accepted. This should be the
> main criterions: usefulness and drawbacks.
> 
> Your argument seem to assume that someone will maliciously try to get a
> crappy demuxer accepted that way. Seems a bit paranoid.

It's an scenario that could happen, like it or not. I'd rather not open the
doors for it.

> 
>> Aside from the "arbitrary markup format" argument, the fact it's disabled by
>> default for security reasons doesn't exactly inspire confidence.
>> It would afaik be a first for an internal module that doesn't depend on
>> external factors.
> 
> Disabled by default was a concession to someone (Paul, IIRC) raising the
> concern of security. It seems very feeble to me: why this format and not
> the many others?

"why this format and not this other" is coincidentally the argument that
will be used after this thing gets committed to justify adding others.

> 
> If this is giving you pause, then let us review all this carefully, fuzz
> it, add regression testing, and enable it by default.
> 
>> Besides, nothing even /muxes/ this format to begin with. The doxy states you
>> should take the ffprobe standard output and edit it by hand or with a script
>> until it looks like something this demuxer can digest.
> 
> Stefano has promised to implement a corresponding muxer (rather easy,
> actually, but a bit of code duplication with ffprobe) and has also
> tried to get ffprobe to output the exact format.

If something is to create these files, it should be ffprobe itself and
not some unrelated muxer that's going to duplicate a lot of code.

> 
>> Which so happens to be what every libav* user has to do for their projects.
>> Write a program using the libraries' public API to read and write files.
>>
>> We have plenty of times refused code people wanted to dump into libavformat
>> and libavcodec because it was the easiest way for them to get what they
>> needed done. There was this absolutely insane youtube-dl "demuxer" that still
>> gives me the chills.
>> We have also plenty of times let things go in, even with several developers
>> against it. Lets try to not do it again.
> 
> This argument mixes a whole lot of different cases together. Each case
> has its own specifics that should be used to judge whether it should
> have been accepted or not.
> 
> Once again: usefulness and drawbacks.
> 
> Having one demuxer of this kind, as powerful as possible, is useful. Do
> you not agree with that?

It might, hence why i suggested this being implemented as a standalone tool
and not that you should drop the whole thing.

> 
> (Having several of them would be less useful.)
> 
> What are the drawbacks you see?

You presented an scenario and use case to dump media streams into a text
file to easily alter them, analyze them and reconstruct them. You then
declared said dumps to be an actual format so they could qualify for an
avformat module that could digest them.

There are no technical drawbacks for this demuxer. This is a concept and
design issue. It's a very specific need so far a single person or two
that's being forced into the libraries because it's the easiest way to
implement it.

This should be written as a standalone tool that uses lavf API to rebuild
the custom xml-like dump with the hex formatted packets as created by
ffprobe. Basically, write ffmpeg's own, more powerful and versatile
version of oggz-dump that uses lavf instead of libogg, meaning the dumps
can be created and rebuilt from and into any lavf supported format.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Le decadi 10 brumaire, an CCXXV, James Almer a écrit :
> Someone will come up with an hexdump demuxer next and call hexdump output
> a multimedia file, if we follow your views. Or an XML parser for a similarly
> arbitrary custom format they happened to devise. Or maybe oggz-dump's output.
> 
> While for the former we can tell them to go away, we wouldn't have many
> arguments against the latter two if this patch goes in. Because telling them
> we don't want their custom format after this one was committed would be
> hypocritical, biased, and a decision as arbitrary as the actual hypothetical
> format.

If these formats are useful, they should be accepted. This should be the
main criterions: usefulness and drawbacks.

Your argument seem to assume that someone will maliciously try to get a
crappy demuxer accepted that way. Seems a bit paranoid.

> Aside from the "arbitrary markup format" argument, the fact it's disabled by
> default for security reasons doesn't exactly inspire confidence.
> It would afaik be a first for an internal module that doesn't depend on
> external factors.

Disabled by default was a concession to someone (Paul, IIRC) raising the
concern of security. It seems very feeble to me: why this format and not
the many others?

If this is giving you pause, then let us review all this carefully, fuzz
it, add regression testing, and enable it by default.

> Besides, nothing even /muxes/ this format to begin with. The doxy states you
> should take the ffprobe standard output and edit it by hand or with a script
> until it looks like something this demuxer can digest.

Stefano has promised to implement a corresponding muxer (rather easy,
actually, but a bit of code duplication with ffprobe) and has also
tried to get ffprobe to output the exact format.

> Which so happens to be what every libav* user has to do for their projects.
> Write a program using the libraries' public API to read and write files.
> 
> We have plenty of times refused code people wanted to dump into libavformat
> and libavcodec because it was the easiest way for them to get what they
> needed done. There was this absolutely insane youtube-dl "demuxer" that still
> gives me the chills.
> We have also plenty of times let things go in, even with several developers
> against it. Lets try to not do it again.

This argument mixes a whole lot of different cases together. Each case
has its own specifics that should be used to judge whether it should
have been accepted or not.

Once again: usefulness and drawbacks.

Having one demuxer of this kind, as powerful as possible, is useful. Do
you not agree with that?

(Having several of them would be less useful.)

What are the drawbacks you see?

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
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
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
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
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
On 10/31/2016 11:57 AM, Nicolas George wrote:
> Jean-Baptiste Kempf:
>> 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.
> 
> Indeed. The same goes for a lot of obscure formats used to decode game
> cutscene data. At least, with ffprobe, we did not have to
> reverse-engineer the format.
> 
>> I understand very well the need,
> 
> Thanks.
> 
>> I disagree it should go inside the
>> (de)muxing library.
> 
> Why? Is it just a matter of taste for you, or do you see practical
> drawbacks?
> 
> James Almer:
>> Afaics, the cons argumented were that this "demuxer" doesn't fit the
>> criteria of a libavformat module.
> 
> I do not know the criteria of a libavformat module. Can you enlighten
> me?
> 
>>  It's not demultiplexing a multimedia
>> file.
> 
> Uh? I think this is exactly what all demuxers do. They can not do
> anything else.
> 
>> It, unless i misread it, is just reconstructing one out of a non-binary
>> dump.
> 
> The "dump" is a file, it contains multimedia data: it is a multimedia
> file.

Someone will come up with an hexdump demuxer next and call hexdump output
a multimedia file, if we follow your views. Or an XML parser for a similarly
arbitrary custom format they happened to devise. Or maybe oggz-dump's output.

While for the former we can tell them to go away, we wouldn't have many
arguments against the latter two if this patch goes in. Because telling them
we don't want their custom format after this one was committed would be
hypocritical, biased, and a decision as arbitrary as the actual hypothetical
format.

> 
>> This should be implemented as a separate tool using libavformat, and
>> not as a libavformat module.
> 
> It could be done. But, once again: why? Is it just a matter of taste for
> you, or do you see practical drawbacks?

Aside from the "arbitrary markup format" argument, the fact it's disabled by
default for security reasons doesn't exactly inspire confidence.
It would afaik be a first for an internal module that doesn't depend on
external factors.

Besides, nothing even /muxes/ this format to begin with. The doxy states you
should take the ffprobe standard output and edit it by hand or with a script
until it looks like something this demuxer can digest.

> 
> Making a tool to build a real file from the text dump allows to get the
> feature without intruding lavf, that is true.
> 
> But it would require exactly the same amount of code to parse the dump,
> plus the code to mux the file: more code, more maintenance.

Which so happens to be what every libav* user has to do for their projects.
Write a program using the libraries' public API to read and write files.

We have plenty of times refused code people wanted to dump into libavformat
and libavcodec because it was the easiest way for them to get what they
needed done. There was this absolutely insane youtube-dl "demuxer" that still
gives me the chills.
We have also plenty of times let things go in, even with several developers
against it. Lets try to not do it again.

> 
> Plus, in terms of debugging, it adds three extra points of failure: the
> muxer for the intermediate format (used in the conversion tool), the
> demuxer for the intermediate format (used in ffmpeg.c) and the
> limitations of the intermediate format itself.
> 
> Regards,
> 
> 
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread wm4
On Mon, 31 Oct 2016 15:20:15 +0100
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.
> 
> 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?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
Jean-Baptiste Kempf:
> 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.

Indeed. The same goes for a lot of obscure formats used to decode game
cutscene data. At least, with ffprobe, we did not have to
reverse-engineer the format.

> I understand very well the need,

Thanks.

>  I disagree it should go inside the
> (de)muxing library.

Why? Is it just a matter of taste for you, or do you see practical
drawbacks?

James Almer:
> Afaics, the cons argumented were that this "demuxer" doesn't fit the
> criteria of a libavformat module.

I do not know the criteria of a libavformat module. Can you enlighten
me?

>   It's not demultiplexing a multimedia
> file.

Uh? I think this is exactly what all demuxers do. They can not do
anything else.

> It, unless i misread it, is just reconstructing one out of a non-binary
> dump.

The "dump" is a file, it contains multimedia data: it is a multimedia
file.

> This should be implemented as a separate tool using libavformat, and
> not as a libavformat module.

It could be done. But, once again: why? Is it just a matter of taste for
you, or do you see practical drawbacks?

Making a tool to build a real file from the text dump allows to get the
feature without intruding lavf, that is true.

But it would require exactly the same amount of code to parse the dump,
plus the code to mux the file: more code, more maintenance.

Plus, in terms of debugging, it adds three extra points of failure: the
muxer for the intermediate format (used in the conversion tool), the
demuxer for the intermediate format (used in ffmpeg.c) and the
limitations of the intermediate format itself.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Stefano Sabatini
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.
-- 
FFmpeg = Frightening & Fancy Marvellous Picky Extroverse Ghost
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
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
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Jean-Baptiste Kempf
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

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread James Almer
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
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Jean-Baptiste Kempf
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.

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread wm4
On Mon, 31 Oct 2016 09:51:10 +0100
Stefano Sabatini  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 
> > > > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > > > Subject: [PATCH] lavf: add ffprobe demuxer
> > > > 
> > > > With several modifications and documentation by Stefano Sabatini
> > > > .
> > > > 
> > > > Signed-off-by: Nicolas George 
> > > > ---
> > > >  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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-31 Thread Stefano Sabatini
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 
> > > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > > Subject: [PATCH] lavf: add ffprobe demuxer
> > > 
> > > With several modifications and documentation by Stefano Sabatini
> > > .
> > > 
> > > Signed-off-by: Nicolas George 
> > > ---
> > >  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.
>From 7f209e27aa33e8f43444e5cfc44c68f664b69e06 Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Sat, 11 Jan 2014 19:42:41 +0100
Subject: [PATCH] lavf: add ffprobe demuxer

With several modifications and documentation by Stefano Sabatini
.

Signed-off-by: Nicolas George 
---
 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 000..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

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-27 Thread Stefano Sabatini
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 
> > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > Subject: [PATCH] lavf: add ffprobe demuxer
> > 
> > With several modifications and documentation by Stefano Sabatini
> > .
> > 
> > Signed-off-by: Nicolas George 
> > ---
> >  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.

Thanks.
-- 
FFmpeg = Fanciful and Forgiving Magic Pitiful Exxagerate Gnome
>From 9eb6595cbcdc7e01b65609f28cda52c798c532e5 Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Sat, 11 Jan 2014 19:42:41 +0100
Subject: [PATCH] lavf: add ffprobe demuxer

With several modifications and documentation by Stefano Sabatini
.

Signed-off-by: Nicolas George 
---
 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 | 429 +++
 7 files changed, 574 insertions(+)
 create mode 100644 doc/ffprobe-format.texi
 create mode 100644 libavformat/ffprobedec.c

diff --git a/configure b/configure
index 481f692..30f4216 100755
--- a/configure
+++ b/configure
@@ -3346,6 +3346,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 000..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 

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-26 Thread wm4
On Wed, 26 Oct 2016 01:46:35 +0200
Michael Niedermayer  wrote:

> On Tue, Oct 25, 2016 at 05:07:34PM +0200, Stefano Sabatini wrote:
> > On date Tuesday 2016-10-18 16:06:47 +0200, Michael Niedermayer encoded:  
> > > On Tue, Oct 18, 2016 at 01:33:27PM +0200, Stefano Sabatini wrote:  
> > > > On date Thursday 2016-10-13 19:46:41 +0200, Stefano Sabatini encoded:  
> > > > > On date Thursday 2016-09-29 21:55:11 +0200, wm4 encoded:  
> > > > [...]  
> > > > > > This seems like a rather special use case. Why does it have a 
> > > > > > demuxer,
> > > > > > and can't be in your own C code using libavcodec/libavformat?
> > > > > >   
> > > > >   
> > > > > > In addition, I think using the ffprobe "format" is an 
> > > > > > overcomplication,
> > > > > > and will justify adding even more stuff to the demuxer, until it's a
> > > > > > similarily complex mess like the ffm demuxer/muxer.  
> > > > > 
> > > > > I'm aware of the issue. OTOH I think the feature per-se is useful, at
> > > > > least for the two mentioned use cases (debugging - especially if
> > > > > coupled with a corresponding muxer), and data streams (generated by
> > > > > script) injection, since it relies on a textual format easily
> > > > > scriptable and editable by hand.
> > > > > 
> > > > > I thus leave the choice to the community. I guess this will require a
> > > > > vote if we cannot find an agreement.
> > > > > 
> > > > > Latest patch in attachment with ffprobe demuxer disabled by default,
> > > > > and extended documentation. (I'm also attaching the ff_get_line2 patch
> > > > > which is used by this new patch).  
> > > > 
> > > > Updated, depends on the ff_get_line2() patch.
> > > > -- 
> > > > FFmpeg = Fanciful and Fundamentalist Magnificient Ponderous Elastic 
> > > > Game  
> > >   
> > > >  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(+)
> > > > 1d8a987564e907802dfd84722e3f5aafa69919ee  
> > > > 0002-lavf-add-ffprobe-demuxer.patch
> > > > From bef4930a2bf280425b5952de0e2281f03897ff7c Mon Sep 17 00:00:00 2001
> > > > From: Nicolas George 
> > > > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > > > Subject: [PATCH] lavf: add ffprobe demuxer
> > > > 
> > > > With several modifications and documentation by Stefano Sabatini
> > > > .
> > > > 
> > > > Signed-off-by: Nicolas George 
> > > > ---  
> > > [...]  
> > > > +static int read_section_packet(AVFormatContext *avf, AVPacket *pkt)
> > > > +{
> > > > +FFprobeContext *ffp = avf->priv_data;
> > > > +uint8_t buf[4096];
> > > > +int ret;
> > > > +AVPacket p;
> > > > +char flags;
> > > > +
> > > > +av_init_packet();
> > > > +p.pos = avio_tell(avf->pb);
> > > > +p.stream_index = -1;
> > > > +p.size = -1;
> > > > +av_bprint_clear(>data);
> > > > +while ((ret = read_section_line(avf, buf, sizeof(buf {
> > > > +int has_time = 0;
> > > > +int64_t pts, dts, duration;
> > > > +char timebuf[1024];
> > > > +
> > > > +if (ret < 0)
> > > > +return ret;
> > > > +if (sscanf(buf, "stream_index=%d", _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;
> > > > +}
> > > > +}
> > > > +
> > > > +#define PARSE_TIME(name_, is_duration) 
> > > >  \
> > > > +sscanf(buf, #name_ "=%"SCNi64, _);  
> > > >  \  
> > >   
> > > > +has_time = sscanf(buf, #name_ "_time=%s", timebuf);
> > > >  \  
> > >   
> >   
> > > %s may be unsafe, not sure if theres a check elsewhere that prevents
> > > writing over the outut buffer  
> > 
> > Now the buffer is big enough, since it has the same size of the read
> > line.
> > -- 
> > FFmpeg = Fanciful and Formidable Maxi Patchable Ecletic Geisha  
> 
> >  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(+)
> > 267580c51d49daf94e73a33175c63ecfed6a0bed  
> > 0002-lavf-add-ffprobe-demuxer.patch
> > From 4e0aac4bc00104483859f9950af2ffb15fea6c12 Mon Sep 17 00:00:00 2001
> > From: Nicolas George 

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-25 Thread Michael Niedermayer
On Tue, Oct 25, 2016 at 05:07:34PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2016-10-18 16:06:47 +0200, Michael Niedermayer encoded:
> > On Tue, Oct 18, 2016 at 01:33:27PM +0200, Stefano Sabatini wrote:
> > > On date Thursday 2016-10-13 19:46:41 +0200, Stefano Sabatini encoded:
> > > > On date Thursday 2016-09-29 21:55:11 +0200, wm4 encoded:
> > > [...]
> > > > > This seems like a rather special use case. Why does it have a demuxer,
> > > > > and can't be in your own C code using libavcodec/libavformat?
> > > > > 
> > > > 
> > > > > In addition, I think using the ffprobe "format" is an 
> > > > > overcomplication,
> > > > > and will justify adding even more stuff to the demuxer, until it's a
> > > > > similarily complex mess like the ffm demuxer/muxer.
> > > > 
> > > > I'm aware of the issue. OTOH I think the feature per-se is useful, at
> > > > least for the two mentioned use cases (debugging - especially if
> > > > coupled with a corresponding muxer), and data streams (generated by
> > > > script) injection, since it relies on a textual format easily
> > > > scriptable and editable by hand.
> > > > 
> > > > I thus leave the choice to the community. I guess this will require a
> > > > vote if we cannot find an agreement.
> > > > 
> > > > Latest patch in attachment with ffprobe demuxer disabled by default,
> > > > and extended documentation. (I'm also attaching the ff_get_line2 patch
> > > > which is used by this new patch).
> > > 
> > > Updated, depends on the ff_get_line2() patch.
> > > -- 
> > > FFmpeg = Fanciful and Fundamentalist Magnificient Ponderous Elastic Game
> > 
> > >  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(+)
> > > 1d8a987564e907802dfd84722e3f5aafa69919ee  
> > > 0002-lavf-add-ffprobe-demuxer.patch
> > > From bef4930a2bf280425b5952de0e2281f03897ff7c Mon Sep 17 00:00:00 2001
> > > From: Nicolas George 
> > > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > > Subject: [PATCH] lavf: add ffprobe demuxer
> > > 
> > > With several modifications and documentation by Stefano Sabatini
> > > .
> > > 
> > > Signed-off-by: Nicolas George 
> > > ---
> > [...]
> > > +static int read_section_packet(AVFormatContext *avf, AVPacket *pkt)
> > > +{
> > > +FFprobeContext *ffp = avf->priv_data;
> > > +uint8_t buf[4096];
> > > +int ret;
> > > +AVPacket p;
> > > +char flags;
> > > +
> > > +av_init_packet();
> > > +p.pos = avio_tell(avf->pb);
> > > +p.stream_index = -1;
> > > +p.size = -1;
> > > +av_bprint_clear(>data);
> > > +while ((ret = read_section_line(avf, buf, sizeof(buf {
> > > +int has_time = 0;
> > > +int64_t pts, dts, duration;
> > > +char timebuf[1024];
> > > +
> > > +if (ret < 0)
> > > +return ret;
> > > +if (sscanf(buf, "stream_index=%d", _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;
> > > +}
> > > +}
> > > +
> > > +#define PARSE_TIME(name_, is_duration)  \
> > > +sscanf(buf, #name_ "=%"SCNi64, _);   \
> > 
> > > +has_time = sscanf(buf, #name_ "_time=%s", timebuf); \
> > 
> 
> > %s may be unsafe, not sure if theres a check elsewhere that prevents
> > writing over the outut buffer
> 
> Now the buffer is big enough, since it has the same size of the read
> line.
> -- 
> FFmpeg = Fanciful and Formidable Maxi Patchable Ecletic Geisha

>  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(+)
> 267580c51d49daf94e73a33175c63ecfed6a0bed  0002-lavf-add-ffprobe-demuxer.patch
> From 4e0aac4bc00104483859f9950af2ffb15fea6c12 Mon Sep 17 00:00:00 2001
> From: Nicolas George 
> Date: Sat, 11 Jan 2014 19:42:41 +0100
> Subject: [PATCH] lavf: add ffprobe demuxer
> 
> With several modifications and documentation by Stefano Sabatini
> .
> 
> Signed-off-by: Nicolas George 
> ---
>  configure|   3 +
>  doc/demuxers.texi|  18 +++
>  doc/ffprobe-format.texi  | 121 ++
>  doc/formats.texi |   

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-25 Thread Stefano Sabatini
On date Tuesday 2016-10-18 16:06:47 +0200, Michael Niedermayer encoded:
> On Tue, Oct 18, 2016 at 01:33:27PM +0200, Stefano Sabatini wrote:
> > On date Thursday 2016-10-13 19:46:41 +0200, Stefano Sabatini encoded:
> > > On date Thursday 2016-09-29 21:55:11 +0200, wm4 encoded:
> > [...]
> > > > This seems like a rather special use case. Why does it have a demuxer,
> > > > and can't be in your own C code using libavcodec/libavformat?
> > > > 
> > > 
> > > > In addition, I think using the ffprobe "format" is an overcomplication,
> > > > and will justify adding even more stuff to the demuxer, until it's a
> > > > similarily complex mess like the ffm demuxer/muxer.
> > > 
> > > I'm aware of the issue. OTOH I think the feature per-se is useful, at
> > > least for the two mentioned use cases (debugging - especially if
> > > coupled with a corresponding muxer), and data streams (generated by
> > > script) injection, since it relies on a textual format easily
> > > scriptable and editable by hand.
> > > 
> > > I thus leave the choice to the community. I guess this will require a
> > > vote if we cannot find an agreement.
> > > 
> > > Latest patch in attachment with ffprobe demuxer disabled by default,
> > > and extended documentation. (I'm also attaching the ff_get_line2 patch
> > > which is used by this new patch).
> > 
> > Updated, depends on the ff_get_line2() patch.
> > -- 
> > FFmpeg = Fanciful and Fundamentalist Magnificient Ponderous Elastic Game
> 
> >  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(+)
> > 1d8a987564e907802dfd84722e3f5aafa69919ee  
> > 0002-lavf-add-ffprobe-demuxer.patch
> > From bef4930a2bf280425b5952de0e2281f03897ff7c Mon Sep 17 00:00:00 2001
> > From: Nicolas George 
> > Date: Sat, 11 Jan 2014 19:42:41 +0100
> > Subject: [PATCH] lavf: add ffprobe demuxer
> > 
> > With several modifications and documentation by Stefano Sabatini
> > .
> > 
> > Signed-off-by: Nicolas George 
> > ---
> [...]
> > +static int read_section_packet(AVFormatContext *avf, AVPacket *pkt)
> > +{
> > +FFprobeContext *ffp = avf->priv_data;
> > +uint8_t buf[4096];
> > +int ret;
> > +AVPacket p;
> > +char flags;
> > +
> > +av_init_packet();
> > +p.pos = avio_tell(avf->pb);
> > +p.stream_index = -1;
> > +p.size = -1;
> > +av_bprint_clear(>data);
> > +while ((ret = read_section_line(avf, buf, sizeof(buf {
> > +int has_time = 0;
> > +int64_t pts, dts, duration;
> > +char timebuf[1024];
> > +
> > +if (ret < 0)
> > +return ret;
> > +if (sscanf(buf, "stream_index=%d", _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;
> > +}
> > +}
> > +
> > +#define PARSE_TIME(name_, is_duration)  \
> > +sscanf(buf, #name_ "=%"SCNi64, _);   \
> 
> > +has_time = sscanf(buf, #name_ "_time=%s", timebuf); \
> 

> %s may be unsafe, not sure if theres a check elsewhere that prevents
> writing over the outut buffer

Now the buffer is big enough, since it has the same size of the read
line.
-- 
FFmpeg = Fanciful and Formidable Maxi Patchable Ecletic Geisha
>From 4e0aac4bc00104483859f9950af2ffb15fea6c12 Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Sat, 11 Jan 2014 19:42:41 +0100
Subject: [PATCH] lavf: add ffprobe demuxer

With several modifications and documentation by Stefano Sabatini
.

Signed-off-by: Nicolas George 
---
 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

diff --git a/configure b/configure
index 1e6834f..7974c12 100755
--- a/configure
+++ b/configure
@@ -3346,6 +3346,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 @@ 

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-25 Thread Stefano Sabatini
On date Wednesday 2016-10-19 17:37:36 +0200, Moritz Barsnick encoded:
> On Tue, Oct 18, 2016 at 13:33:27 +0200, Stefano Sabatini wrote:
> > > Latest patch in attachment with ffprobe demuxer disabled by default,
> > > and extended documentation. (I'm also attaching the ff_get_line2 patch
> > > which is used by this new patch).
> > Updated, depends on the ff_get_line2() patch.
> [...]
> > +The ffprobe demuxer format is inspired by the output generated by the
> > +ffprobe default format.
> 

> Unfortunately, you dropped the ffprobe command line which can be used
> to create exactly that format. I found that quite practical.
> (Presumably a mix of ffprobe "-show_format", "-show_streams",
> "-show_packets -show_data"?) But I think it was incorrect anyway in the
> previous patches. ;-)

The fact is that without the -show_headers_first option it is not
possible to generate that output anymore (and you need to edit the
output generated by ffprobe anyway). I'm going to add a muxer for
that.

> > +A ffprobe demuxer file might look like this:
> ^ nit: An

Fixed, thanks.
-- 
FFmpeg = Fascinating and Friendly Majestic Political Enhanced Governor
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-19 Thread Moritz Barsnick
On Tue, Oct 18, 2016 at 13:33:27 +0200, Stefano Sabatini wrote:
> > Latest patch in attachment with ffprobe demuxer disabled by default,
> > and extended documentation. (I'm also attaching the ff_get_line2 patch
> > which is used by this new patch).
> Updated, depends on the ff_get_line2() patch.
[...]
> +The ffprobe demuxer format is inspired by the output generated by the
> +ffprobe default format.

Unfortunately, you dropped the ffprobe command line which can be used
to create exactly that format. I found that quite practical.
(Presumably a mix of ffprobe "-show_format", "-show_streams",
"-show_packets -show_data"?) But I think it was incorrect anyway in the
previous patches. ;-)

> +A ffprobe demuxer file might look like this:
^ nit: An

Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-18 Thread Michael Niedermayer
On Tue, Oct 18, 2016 at 01:33:27PM +0200, Stefano Sabatini wrote:
> On date Thursday 2016-10-13 19:46:41 +0200, Stefano Sabatini encoded:
> > On date Thursday 2016-09-29 21:55:11 +0200, wm4 encoded:
> [...]
> > > This seems like a rather special use case. Why does it have a demuxer,
> > > and can't be in your own C code using libavcodec/libavformat?
> > > 
> > 
> > > In addition, I think using the ffprobe "format" is an overcomplication,
> > > and will justify adding even more stuff to the demuxer, until it's a
> > > similarily complex mess like the ffm demuxer/muxer.
> > 
> > I'm aware of the issue. OTOH I think the feature per-se is useful, at
> > least for the two mentioned use cases (debugging - especially if
> > coupled with a corresponding muxer), and data streams (generated by
> > script) injection, since it relies on a textual format easily
> > scriptable and editable by hand.
> > 
> > I thus leave the choice to the community. I guess this will require a
> > vote if we cannot find an agreement.
> > 
> > Latest patch in attachment with ffprobe demuxer disabled by default,
> > and extended documentation. (I'm also attaching the ff_get_line2 patch
> > which is used by this new patch).
> 
> Updated, depends on the ff_get_line2() patch.
> -- 
> FFmpeg = Fanciful and Fundamentalist Magnificient Ponderous Elastic Game

>  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(+)
> 1d8a987564e907802dfd84722e3f5aafa69919ee  0002-lavf-add-ffprobe-demuxer.patch
> From bef4930a2bf280425b5952de0e2281f03897ff7c Mon Sep 17 00:00:00 2001
> From: Nicolas George 
> Date: Sat, 11 Jan 2014 19:42:41 +0100
> Subject: [PATCH] lavf: add ffprobe demuxer
> 
> With several modifications and documentation by Stefano Sabatini
> .
> 
> Signed-off-by: Nicolas George 
> ---
[...]
> +static int read_section_packet(AVFormatContext *avf, AVPacket *pkt)
> +{
> +FFprobeContext *ffp = avf->priv_data;
> +uint8_t buf[4096];
> +int ret;
> +AVPacket p;
> +char flags;
> +
> +av_init_packet();
> +p.pos = avio_tell(avf->pb);
> +p.stream_index = -1;
> +p.size = -1;
> +av_bprint_clear(>data);
> +while ((ret = read_section_line(avf, buf, sizeof(buf {
> +int has_time = 0;
> +int64_t pts, dts, duration;
> +char timebuf[1024];
> +
> +if (ret < 0)
> +return ret;
> +if (sscanf(buf, "stream_index=%d", _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;
> +}
> +}
> +
> +#define PARSE_TIME(name_, is_duration)  \
> +sscanf(buf, #name_ "=%"SCNi64, _);   \

> +has_time = sscanf(buf, #name_ "_time=%s", timebuf); \

%s may be unsafe, not sure if theres a check elsewhere that prevents
writing over the outut buffer

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-18 Thread Stefano Sabatini
On date Thursday 2016-10-13 19:46:41 +0200, Stefano Sabatini encoded:
> On date Thursday 2016-09-29 21:55:11 +0200, wm4 encoded:
[...]
> > This seems like a rather special use case. Why does it have a demuxer,
> > and can't be in your own C code using libavcodec/libavformat?
> > 
> 
> > In addition, I think using the ffprobe "format" is an overcomplication,
> > and will justify adding even more stuff to the demuxer, until it's a
> > similarily complex mess like the ffm demuxer/muxer.
> 
> I'm aware of the issue. OTOH I think the feature per-se is useful, at
> least for the two mentioned use cases (debugging - especially if
> coupled with a corresponding muxer), and data streams (generated by
> script) injection, since it relies on a textual format easily
> scriptable and editable by hand.
> 
> I thus leave the choice to the community. I guess this will require a
> vote if we cannot find an agreement.
> 
> Latest patch in attachment with ffprobe demuxer disabled by default,
> and extended documentation. (I'm also attaching the ff_get_line2 patch
> which is used by this new patch).

Updated, depends on the ff_get_line2() patch.
-- 
FFmpeg = Fanciful and Fundamentalist Magnificient Ponderous Elastic Game
>From bef4930a2bf280425b5952de0e2281f03897ff7c Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Sat, 11 Jan 2014 19:42:41 +0100
Subject: [PATCH] lavf: add ffprobe demuxer

With several modifications and documentation by Stefano Sabatini
.

Signed-off-by: Nicolas George 
---
 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

diff --git a/configure b/configure
index 96f575f..65995e7 100755
--- a/configure
+++ b/configure
@@ -3340,6 +3340,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 000..fa34f5e
--- /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.

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-18 Thread Stefano Sabatini
On date Thursday 2016-09-29 21:02:14 +0100, Josh de Kock encoded:
[...]
> Are you sure this cannot be done from outside of libav*? It really
> seems like something which wouldn't actually be very useful for
> anyone else.

It can be done outside of libav* (reading from a custom format, and
then converting to one of the supported output formats), but this
would imply another processing step.

> It's also just another hack in for a non-standardised,
> exclusive format. Lavf is really not a good fit for this and it
> should be done outside of libav*.

See my previous replies. I think there are both advantages and
disadvantages for integrating this format, and it should be probably
disabled by default for security concerns.
-- 
FFmpeg = Fierce & Foolish Murdering Probing Enhancing Goblin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-18 Thread Stefano Sabatini
On date Friday 2016-10-14 00:11:49 +0200, Moritz Barsnick encoded:
> On Thu, Oct 13, 2016 at 19:46:41 +0200, Stefano Sabatini wrote:
> > +In particular, can be also used to inject data stream generated by
>   ^ it can
> 
> > +Unrecognized values are discarded (this allows the demuxer to accept
> > +the output generated by @command{ffprobe} without further
> > +modifications.
> ^) (missing closing bracket)
> 
> > +the codec name (the name must be an accepted FFmpeg codec name
>  ^) (missing 
> closing bracket)
> 
> > +@item duration_tim
>  ^duration_time
>  

> > +av_log(avf, AV_LOG_ERROR, "Invalid " #name_ " time 
> > specification '%s' for data packet\n", \
> ^ Is 
> there a space too much here ("duration_ time")?

No, this is correct as is (it renders to "duration time", the "_" is
part of the variable name).

Will send an update soon, thanks.
-- 
FFmpeg = Formidable and Foolish Magnificient Power Explosive Geisha
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-13 Thread Moritz Barsnick
On Thu, Oct 13, 2016 at 19:46:41 +0200, Stefano Sabatini wrote:
> +In particular, can be also used to inject data stream generated by
  ^ it can

> +Unrecognized values are discarded (this allows the demuxer to accept
> +the output generated by @command{ffprobe} without further
> +modifications.
^) (missing closing bracket)

> +the codec name (the name must be an accepted FFmpeg codec name
 ^) (missing 
closing bracket)

> +@item duration_tim
 ^duration_time
 
> +av_log(avf, AV_LOG_ERROR, "Invalid " #name_ " time 
> specification '%s' for data packet\n", \
^ Is there 
a space too much here ("duration_ time")?


Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-10-13 Thread Stefano Sabatini
On date Thursday 2016-09-29 21:55:11 +0200, wm4 encoded:
> On Thu, 29 Sep 2016 01:21:01 +0200
> Stefano Sabatini  wrote:
> 
> > On date Monday 2016-09-26 18:55:47 +0200, wm4 encoded:
> > > On Sun, 25 Sep 2016 19:32:37 +0200
> > > Stefano Sabatini  wrote:  
> > [...]
> > > > > > My use case: I need to build a data stream with scripting/manual
> > > > > > editing. Since I don't want to have to rely on a binary format 
> > > > > > (which
> > > > > > is not ideal for that use case) I needed a format simple enough to 
> > > > > > be
> > > > > > written and analysed without special tools, but with a simple text
> > > > > > editor.
> > > > > 
> > > > > I still don't get it. Your description makes me think of something 
> > > > > like
> > > > > EDL, but that doesn't seem to have anything to do with it.
> > > > 
> > > > EDL as "Edit Decision List"? No I don't think this is related at
> > > > all.
> > > > 
> > > > Suppose you want to inject a data stream, you have the data and you
> > > > know where to insert it given its PTS. With this muxer you can
> > > > handcraft a textual file in the ffprobe_default format and use ffmpeg
> > > > to inject the new data stream.  
> > >   
> > 
> > > What's a "data stream"?  
> > 
> > A stream consisting of generic data, that is a stream with type
> > AV_MEDIA_TYPE_DATA.
> > 
> > > Inject into what?  
> > 
> > Interleave the stream with other media streams (audio, video,
> > etc.). This can be done via remuxing (no need for transcoding).
> > 
> > > And why?  
> > 
> > Data stream could be used to insert custom information for
> > post-processing (e.g. they might contain advertising information).
> 
> This seems like a rather special use case. Why does it have a demuxer,
> and can't be in your own C code using libavcodec/libavformat?
> 

> In addition, I think using the ffprobe "format" is an overcomplication,
> and will justify adding even more stuff to the demuxer, until it's a
> similarily complex mess like the ffm demuxer/muxer.

I'm aware of the issue. OTOH I think the feature per-se is useful, at
least for the two mentioned use cases (debugging - especially if
coupled with a corresponding muxer), and data streams (generated by
script) injection, since it relies on a textual format easily
scriptable and editable by hand.

I thus leave the choice to the community. I guess this will require a
vote if we cannot find an agreement.

Latest patch in attachment with ffprobe demuxer disabled by default,
and extended documentation. (I'm also attaching the ff_get_line2 patch
which is used by this new patch).
-- 
FFmpeg = Fierce Frenzy Magnificient Plastic Explosive Gadget
>From 4e6a56ab7f59bc9811881c5020e005c3f2fb5b4c Mon Sep 17 00:00:00 2001
From: Stefano Sabatini 
Date: Thu, 13 Oct 2016 16:36:30 +0200
Subject: [PATCH] lavf/aviobuf: add ff_get_line2() variant

This allows to probe if the read line was partially discarded.
---
 libavformat/aviobuf.c  | 10 +-
 libavformat/internal.h | 14 ++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 134d627..28183b4 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -764,18 +764,26 @@ unsigned int avio_rb32(AVIOContext *s)
 
 int ff_get_line(AVIOContext *s, char *buf, int maxlen)
 {
-int i = 0;
+return ff_get_line2(s, buf, maxlen, NULL);
+}
+
+int ff_get_line2(AVIOContext *s, char *buf, int maxlen, int *readlen)
+{
+int i = 0, j = 0;
 char c;
 
 do {
 c = avio_r8(s);
 if (c && i < maxlen-1)
 buf[i++] = c;
+j++;
 } while (c != '\n' && c != '\r' && c);
 if (c == '\r' && avio_r8(s) != '\n' && !avio_feof(s))
 avio_skip(s, -1);
 
 buf[i] = 0;
+if (readlen)
+*readlen = j;
 return i;
 }
 
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 49244fa..cffc787 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -274,6 +274,20 @@ void ff_put_v(AVIOContext *bc, uint64_t val);
  */
 int ff_get_line(AVIOContext *s, char *buf, int maxlen);
 
+/**
+ * Read a whole line of text from AVIOContext. Stop reading after reaching
+ * either a \\n, a \\0 or EOF. The returned string is always \\0-terminated,
+ * and may be truncated if the buffer is too small.
+ *
+ * @param s the read-only AVIOContext
+ * @param buf buffer to store the read line
+ * @param maxlen size of the buffer
+ * @param readlen length of the read line, including the terminating newline character
+ * @return the length of the string written in the buffer, not including the
+ * final \\0
+ */
+int ff_get_line2(AVIOContext *s, char *buf, int maxlen, int *readlen);
+
 #define SPACE_CHARS " \t\r\n"
 
 /**
-- 
1.9.1

>From 87243602a7a3775e4b2bf2c6f9f3afcd24fefdea Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Sat, 11 Jan 2014 19:42:41 +0100
Subject: [PATCH] lavf: add ffprobe demuxer


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-29 Thread Nicolas George
L'octidi 8 vendémiaire, an CCXXV, wm4 a écrit :
> This seems like a rather special use case. Why does it have a demuxer,
> and can't be in your own C code using libavcodec/libavformat?

Of course, it can. It just takes more effort overall. You do not believe
that Stefano and I both started working on a similar project on a whim, do
you? We did it because we felt it would gain time in the long run. Once the
demuxer is there, it is ready for all uses.

I very often have samples triggering bugs that I want to tweak to see what
is causing the bug exactly: timestamps, order of packets, even the payload.
Editing them with a powerful text editor or a perl one-liner beats anything
in terms of efficiency to do so.

> In addition, I think using the ffprobe "format" is an overcomplication,
> and will justify adding even more stuff to the demuxer, until it's a
> similarily complex mess like the ffm demuxer/muxer.

This looks like a "slippery slope" fallacy.

Since it is clearly useful almost only for developers doing debugging, I
would not mind having no probe function or even disabled by default, making
it a non-issue security-wise. Beyond that, I have yet to see an actual
argument against it.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-29 Thread Josh de Kock



On 29/09/2016 00:21, Stefano Sabatini wrote:

On date Monday 2016-09-26 18:55:47 +0200, wm4 encoded:

On Sun, 25 Sep 2016 19:32:37 +0200
Stefano Sabatini  wrote:

[...]

My use case: I need to build a data stream with scripting/manual
editing. Since I don't want to have to rely on a binary format (which
is not ideal for that use case) I needed a format simple enough to be
written and analysed without special tools, but with a simple text
editor.


I still don't get it. Your description makes me think of something like
EDL, but that doesn't seem to have anything to do with it.


EDL as "Edit Decision List"? No I don't think this is related at
all.

Suppose you want to inject a data stream, you have the data and you
know where to insert it given its PTS. With this muxer you can
handcraft a textual file in the ffprobe_default format and use ffmpeg
to inject the new data stream.





What's a "data stream"?


A stream consisting of generic data, that is a stream with type
AV_MEDIA_TYPE_DATA.


Inject into what?


Interleave the stream with other media streams (audio, video,
etc.). This can be done via remuxing (no need for transcoding).


And why?


Data stream could be used to insert custom information for
post-processing (e.g. they might contain advertising information).



Are you sure this cannot be done from outside of libav*? It really seems 
like something which wouldn't actually be very useful for anyone else. 
It's also just another hack in for a non-standardised, exclusive format. 
Lavf is really not a good fit for this and it should be done outside of 
libav*.


--
Josh
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-29 Thread wm4
On Thu, 29 Sep 2016 01:21:01 +0200
Stefano Sabatini  wrote:

> On date Monday 2016-09-26 18:55:47 +0200, wm4 encoded:
> > On Sun, 25 Sep 2016 19:32:37 +0200
> > Stefano Sabatini  wrote:  
> [...]
> > > > > My use case: I need to build a data stream with scripting/manual
> > > > > editing. Since I don't want to have to rely on a binary format (which
> > > > > is not ideal for that use case) I needed a format simple enough to be
> > > > > written and analysed without special tools, but with a simple text
> > > > > editor.
> > > > 
> > > > I still don't get it. Your description makes me think of something like
> > > > EDL, but that doesn't seem to have anything to do with it.
> > > 
> > > EDL as "Edit Decision List"? No I don't think this is related at
> > > all.
> > > 
> > > Suppose you want to inject a data stream, you have the data and you
> > > know where to insert it given its PTS. With this muxer you can
> > > handcraft a textual file in the ffprobe_default format and use ffmpeg
> > > to inject the new data stream.  
> >   
> 
> > What's a "data stream"?  
> 
> A stream consisting of generic data, that is a stream with type
> AV_MEDIA_TYPE_DATA.
> 
> > Inject into what?  
> 
> Interleave the stream with other media streams (audio, video,
> etc.). This can be done via remuxing (no need for transcoding).
> 
> > And why?  
> 
> Data stream could be used to insert custom information for
> post-processing (e.g. they might contain advertising information).

This seems like a rather special use case. Why does it have a demuxer,
and can't be in your own C code using libavcodec/libavformat?

In addition, I think using the ffprobe "format" is an overcomplication,
and will justify adding even more stuff to the demuxer, until it's a
similarily complex mess like the ffm demuxer/muxer.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-26 Thread wm4
On Sun, 25 Sep 2016 19:32:37 +0200
Stefano Sabatini  wrote:

> On date Saturday 2016-09-24 15:21:11 +0200, wm4 encoded:
> > On Fri, 23 Sep 2016 19:46:16 +0200
> > Stefano Sabatini  wrote:
> >   
> > > On date Friday 2016-09-23 09:34:19 +0200, wm4 encoded:  
> > > > On Thu, 22 Sep 2016 18:50:27 +0200
> > > > Stefano Sabatini  wrote:
> > > [...]  
> > > > > Ping. I'd like to commit this if there are no objections. Also,
> > > > > possibly add a muxer to generate output directly readable by the
> > > > > demuxer (this way we don't need ffprobe for generating the output, and
> > > > > we avoid the ordering issue).
> > > > 
> > > > I'm sorry to jump in as a nay-sayer, but it looks like a quite fishy
> > > > concept and I still don't get why it's supposed to be needed. The
> > > > documentation also doesn't say what this is supposed to be useful for.  
> > > >   
> > > 
> > > I'll extend the documentation to add some possibly useful use cases.
> > > 
> > > My use case: I need to build a data stream with scripting/manual
> > > editing. Since I don't want to have to rely on a binary format (which
> > > is not ideal for that use case) I needed a format simple enough to be
> > > written and analysed without special tools, but with a simple text
> > > editor.  
> > 
> > I still don't get it. Your description makes me think of something like
> > EDL, but that doesn't seem to have anything to do with it.  
> 
> EDL as "Edit Decision List"? No I don't think this is related at
> all.
> 
> Suppose you want to inject a data stream, you have the data and you
> know where to insert it given its PTS. With this muxer you can
> handcraft a textual file in the ffprobe_default format and use ffmpeg
> to inject the new data stream.

What's a "data stream"? Inject into what? And why?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-25 Thread Stefano Sabatini
On date Saturday 2016-09-24 15:21:11 +0200, wm4 encoded:
> On Fri, 23 Sep 2016 19:46:16 +0200
> Stefano Sabatini  wrote:
> 
> > On date Friday 2016-09-23 09:34:19 +0200, wm4 encoded:
> > > On Thu, 22 Sep 2016 18:50:27 +0200
> > > Stefano Sabatini  wrote:  
> > [...]
> > > > Ping. I'd like to commit this if there are no objections. Also,
> > > > possibly add a muxer to generate output directly readable by the
> > > > demuxer (this way we don't need ffprobe for generating the output, and
> > > > we avoid the ordering issue).  
> > > 
> > > I'm sorry to jump in as a nay-sayer, but it looks like a quite fishy
> > > concept and I still don't get why it's supposed to be needed. The
> > > documentation also doesn't say what this is supposed to be useful for.  
> > 
> > I'll extend the documentation to add some possibly useful use cases.
> > 
> > My use case: I need to build a data stream with scripting/manual
> > editing. Since I don't want to have to rely on a binary format (which
> > is not ideal for that use case) I needed a format simple enough to be
> > written and analysed without special tools, but with a simple text
> > editor.
> 
> I still don't get it. Your description makes me think of something like
> EDL, but that doesn't seem to have anything to do with it.

EDL as "Edit Decision List"? No I don't think this is related at
all.

Suppose you want to inject a data stream, you have the data and you
know where to insert it given its PTS. With this muxer you can
handcraft a textual file in the ffprobe_default format and use ffmpeg
to inject the new data stream.
-- 
FFmpeg = Furious and Fancy Mystic Peaceful Exciting Guru
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-24 Thread Carl Eugen Hoyos
2016-09-24 15:21 GMT+02:00 wm4 :
> On Fri, 23 Sep 2016 22:27:00 +0200
> Carl Eugen Hoyos  wrote:
>
>> 2016-09-23 19:46 GMT+02:00 Stefano Sabatini :
>> > On date Friday 2016-09-23 09:34:19 +0200, wm4 encoded:
>> >> at least it shouldn't be enabled by default.
>> >
>> > I understand the security concerns, and I have no objections
>> > against disabling this by default if developers prefer this way.
>>
>> Disabling features in configure by default is a bad idea,
>> a demuxer that has to be forced by the user is ok imo.
>
> Yet it's ok if external libs need to be enabled manually by the user?

It's neither ok nor not ok but how FFmpeg does it for non-system
libraries.

And I am against changing our defaults unless they are
unreasonable or even buggy: We would brake every single
build script when switching to auto-detection of external
libraries.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-24 Thread wm4
On Fri, 23 Sep 2016 22:27:00 +0200
Carl Eugen Hoyos  wrote:

> 2016-09-23 19:46 GMT+02:00 Stefano Sabatini :
> > On date Friday 2016-09-23 09:34:19 +0200, wm4 encoded:  
> >> at least it shouldn't be enabled by default.  
> >
> > I understand the security concerns, and I have no objections
> > against disabling this by default if developers prefer this way.  
> 
> Disabling features in configure by default is a bad idea,
> a demuxer that has to be forced by the user is ok imo.

Yet it's ok if external libs need to be enabled manually by the user?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-24 Thread wm4
On Fri, 23 Sep 2016 19:46:16 +0200
Stefano Sabatini  wrote:

> On date Friday 2016-09-23 09:34:19 +0200, wm4 encoded:
> > On Thu, 22 Sep 2016 18:50:27 +0200
> > Stefano Sabatini  wrote:  
> [...]
> > > Ping. I'd like to commit this if there are no objections. Also,
> > > possibly add a muxer to generate output directly readable by the
> > > demuxer (this way we don't need ffprobe for generating the output, and
> > > we avoid the ordering issue).  
> > 
> > I'm sorry to jump in as a nay-sayer, but it looks like a quite fishy
> > concept and I still don't get why it's supposed to be needed. The
> > documentation also doesn't say what this is supposed to be useful for.  
> 
> I'll extend the documentation to add some possibly useful use cases.
> 
> My use case: I need to build a data stream with scripting/manual
> editing. Since I don't want to have to rely on a binary format (which
> is not ideal for that use case) I needed a format simple enough to be
> written and analysed without special tools, but with a simple text
> editor.

I still don't get it. Your description makes me think of something like
EDL, but that doesn't seem to have anything to do with it.

> Also, if coupled with a muxer it allows to create an output, tweak it
> (e.g. to change the timestamps) and feed it to FFmpeg, which might be
> useful from time to time to test/debug specific features.
> 
> The alternative might be to build a tool for converting to
> custom-format to a "binary" format accepted by FFmpeg, but I want
> to avoid the need for an additional tool.
> 
> This format is my second attempt after the fftextdata format, which
> was regarded too limited. This one is based on the ffprobe default
> output (although it's simplified).
> 
> > It's also potentially a security issue (lots of text parsing, and can
> > construct inputs which might be tricky for API users).
> > 
> > I probably can't stop you from pushing this, but at least it shouldn't
> > be enabled by default.  
> 
> I understand the security concerns, and I have no objections against
> disabling this by default if developers prefer this way.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-23 Thread Carl Eugen Hoyos
2016-09-23 19:46 GMT+02:00 Stefano Sabatini :
> On date Friday 2016-09-23 09:34:19 +0200, wm4 encoded:
>> at least it shouldn't be enabled by default.
>
> I understand the security concerns, and I have no objections
> against disabling this by default if developers prefer this way.

Disabling features in configure by default is a bad idea,
a demuxer that has to be forced by the user is ok imo.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-23 Thread Stefano Sabatini
On date Friday 2016-09-23 09:34:19 +0200, wm4 encoded:
> On Thu, 22 Sep 2016 18:50:27 +0200
> Stefano Sabatini  wrote:
[...]
> > Ping. I'd like to commit this if there are no objections. Also,
> > possibly add a muxer to generate output directly readable by the
> > demuxer (this way we don't need ffprobe for generating the output, and
> > we avoid the ordering issue).
> 
> I'm sorry to jump in as a nay-sayer, but it looks like a quite fishy
> concept and I still don't get why it's supposed to be needed. The
> documentation also doesn't say what this is supposed to be useful for.

I'll extend the documentation to add some possibly useful use cases.

My use case: I need to build a data stream with scripting/manual
editing. Since I don't want to have to rely on a binary format (which
is not ideal for that use case) I needed a format simple enough to be
written and analysed without special tools, but with a simple text
editor.

Also, if coupled with a muxer it allows to create an output, tweak it
(e.g. to change the timestamps) and feed it to FFmpeg, which might be
useful from time to time to test/debug specific features.

The alternative might be to build a tool for converting to
custom-format to a "binary" format accepted by FFmpeg, but I want
to avoid the need for an additional tool.

This format is my second attempt after the fftextdata format, which
was regarded too limited. This one is based on the ffprobe default
output (although it's simplified).

> It's also potentially a security issue (lots of text parsing, and can
> construct inputs which might be tricky for API users).
> 
> I probably can't stop you from pushing this, but at least it shouldn't
> be enabled by default.

I understand the security concerns, and I have no objections against
disabling this by default if developers prefer this way.
-- 
FFmpeg = Freak Frightening Merciless Pacific Erroneous Glue
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-23 Thread wm4
On Thu, 22 Sep 2016 18:50:27 +0200
Stefano Sabatini  wrote:

> On date Sunday 2016-09-18 15:28:45 +0200, Stefano Sabatini encoded:
> > On date Saturday 2016-09-17 18:42:35 +0200, Paul B Mahol encoded:  
> > > On 9/17/16, Stefano Sabatini  wrote:  
> > > > On date Sunday 2016-09-04 23:25:56 +0200, Michael Niedermayer encoded:  
> > > >> On Sun, Sep 04, 2016 at 06:24:37PM +0200, Stefano Sabatini wrote:  
> > > >> > From: Nicolas George 
> > > >> >
> > > >> > With several modifications and documentation by Stefano Sabatini
> > > >> > .
> > > >> >
> > > >> > Signed-off-by: Nicolas George 
> > > >> > ---
> > > >> >  doc/ffprobe-format.texi  | 130 
> > > >> >  libavformat/Makefile |   1 +
> > > >> >  libavformat/allformats.c |   2 +
> > > >> >  libavformat/ffprobedec.c | 397
> > > >> > +++
> > > >> >  4 files changed, 530 insertions(+)
> > > >> >  create mode 100644 doc/ffprobe-format.texi
> > > >> >  create mode 100644 libavformat/ffprobedec.c  
> > > >>
> > > >> this seems not to apply cleanly
> > > >>
> > > >> can i pick this from some git repo ? (should work better than patch
> > > >> with unavailable ancestors)  
> > > >
> > > > Updated, this should apply cleanly on master.  
> > > 
> > > Why we need this hack?  
> > 
> > My use case: I need to inject a metadata stream using the ff*
> > tools. Metadata must be specified in a textual format. I already
> > designed and implemented the fftextdata format, but that was regarded
> > too limited.
> > 
> > Example:
> > ffmpeg -i input.mp4 -copyts -i data.ffprobe -map 0:v -map 0:a -map 1:0 
> > -codec:d copy input+data.ts
> > 
> > With this format we allow to cover my use case, and it provides a more
> > generic format for such purposes (since you can specify multiple
> > streams). Also, it's inspired by the ffprobe output, so it can be used
> > together with ffprobe (via simple text editing) to generate files
> > which can be edited and feed to ffmpeg.
> > 
> > It would be possible to use any binary format for injecting the
> > metadata, but this approach is not very scriptable. Alternatively a
> > custom program to convert a custom textual format to any binary format
> > we support, but this would imply an additional step I want to avoid.
> > 
> > That said, if someone want/can propose an alternative path to this I'm
> > very open to suggestions.  
> 
> Ping. I'd like to commit this if there are no objections. Also,
> possibly add a muxer to generate output directly readable by the
> demuxer (this way we don't need ffprobe for generating the output, and
> we avoid the ordering issue).

I'm sorry to jump in as a nay-sayer, but it looks like a quite fishy
concept and I still don't get why it's supposed to be needed. The
documentation also doesn't say what this is supposed to be useful for.

It's also potentially a security issue (lots of text parsing, and can
construct inputs which might be tricky for API users).

I probably can't stop you from pushing this, but at least it shouldn't
be enabled by default.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-22 Thread Stefano Sabatini
On date Sunday 2016-09-18 15:28:45 +0200, Stefano Sabatini encoded:
> On date Saturday 2016-09-17 18:42:35 +0200, Paul B Mahol encoded:
> > On 9/17/16, Stefano Sabatini  wrote:
> > > On date Sunday 2016-09-04 23:25:56 +0200, Michael Niedermayer encoded:
> > >> On Sun, Sep 04, 2016 at 06:24:37PM +0200, Stefano Sabatini wrote:
> > >> > From: Nicolas George 
> > >> >
> > >> > With several modifications and documentation by Stefano Sabatini
> > >> > .
> > >> >
> > >> > Signed-off-by: Nicolas George 
> > >> > ---
> > >> >  doc/ffprobe-format.texi  | 130 
> > >> >  libavformat/Makefile |   1 +
> > >> >  libavformat/allformats.c |   2 +
> > >> >  libavformat/ffprobedec.c | 397
> > >> > +++
> > >> >  4 files changed, 530 insertions(+)
> > >> >  create mode 100644 doc/ffprobe-format.texi
> > >> >  create mode 100644 libavformat/ffprobedec.c
> > >>
> > >> this seems not to apply cleanly
> > >>
> > >> can i pick this from some git repo ? (should work better than patch
> > >> with unavailable ancestors)
> > >
> > > Updated, this should apply cleanly on master.
> > 
> > Why we need this hack?
> 
> My use case: I need to inject a metadata stream using the ff*
> tools. Metadata must be specified in a textual format. I already
> designed and implemented the fftextdata format, but that was regarded
> too limited.
> 
> Example:
> ffmpeg -i input.mp4 -copyts -i data.ffprobe -map 0:v -map 0:a -map 1:0 
> -codec:d copy input+data.ts
> 
> With this format we allow to cover my use case, and it provides a more
> generic format for such purposes (since you can specify multiple
> streams). Also, it's inspired by the ffprobe output, so it can be used
> together with ffprobe (via simple text editing) to generate files
> which can be edited and feed to ffmpeg.
> 
> It would be possible to use any binary format for injecting the
> metadata, but this approach is not very scriptable. Alternatively a
> custom program to convert a custom textual format to any binary format
> we support, but this would imply an additional step I want to avoid.
> 
> That said, if someone want/can propose an alternative path to this I'm
> very open to suggestions.

Ping. I'd like to commit this if there are no objections. Also,
possibly add a muxer to generate output directly readable by the
demuxer (this way we don't need ffprobe for generating the output, and
we avoid the ordering issue).
-- 
FFmpeg = Fundamental and Fundamental Mere Ponderous Elastic Gargoyle
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-18 Thread Stefano Sabatini
On date Saturday 2016-09-17 18:42:35 +0200, Paul B Mahol encoded:
> On 9/17/16, Stefano Sabatini  wrote:
> > On date Sunday 2016-09-04 23:25:56 +0200, Michael Niedermayer encoded:
> >> On Sun, Sep 04, 2016 at 06:24:37PM +0200, Stefano Sabatini wrote:
> >> > From: Nicolas George 
> >> >
> >> > With several modifications and documentation by Stefano Sabatini
> >> > .
> >> >
> >> > Signed-off-by: Nicolas George 
> >> > ---
> >> >  doc/ffprobe-format.texi  | 130 
> >> >  libavformat/Makefile |   1 +
> >> >  libavformat/allformats.c |   2 +
> >> >  libavformat/ffprobedec.c | 397
> >> > +++
> >> >  4 files changed, 530 insertions(+)
> >> >  create mode 100644 doc/ffprobe-format.texi
> >> >  create mode 100644 libavformat/ffprobedec.c
> >>
> >> this seems not to apply cleanly
> >>
> >> can i pick this from some git repo ? (should work better than patch
> >> with unavailable ancestors)
> >
> > Updated, this should apply cleanly on master.
> 
> Why we need this hack?

My use case: I need to inject a metadata stream using the ff*
tools. Metadata must be specified in a textual format. I already
designed and implemented the fftextdata format, but that was regarded
too limited.

Example:
ffmpeg -i input.mp4 -copyts -i data.ffprobe -map 0:v -map 0:a -map 1:0 -codec:d 
copy input+data.ts

With this format we allow to cover my use case, and it provides a more
generic format for such purposes (since you can specify multiple
streams). Also, it's inspired by the ffprobe output, so it can be used
together with ffprobe (via simple text editing) to generate files
which can be edited and feed to ffmpeg.

It would be possible to use any binary format for injecting the
metadata, but this approach is not very scriptable. Alternatively a
custom program to convert a custom textual format to any binary format
we support, but this would imply an additional step I want to avoid.

That said, if someone want/can propose an alternative path to this I'm
very open to suggestions.
-- 
FFmpeg = Fascinating Fancy Magical Portentous Empowered Guru
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-17 Thread Nicolas George
Le jour du Génie, an CCXXIV, Paul B Mahol a écrit :
> Why we need this hack?

I think I already gave an answer.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-17 Thread Paul B Mahol
On 9/17/16, Stefano Sabatini  wrote:
> On date Sunday 2016-09-04 23:25:56 +0200, Michael Niedermayer encoded:
>> On Sun, Sep 04, 2016 at 06:24:37PM +0200, Stefano Sabatini wrote:
>> > From: Nicolas George 
>> >
>> > With several modifications and documentation by Stefano Sabatini
>> > .
>> >
>> > Signed-off-by: Nicolas George 
>> > ---
>> >  doc/ffprobe-format.texi  | 130 
>> >  libavformat/Makefile |   1 +
>> >  libavformat/allformats.c |   2 +
>> >  libavformat/ffprobedec.c | 397
>> > +++
>> >  4 files changed, 530 insertions(+)
>> >  create mode 100644 doc/ffprobe-format.texi
>> >  create mode 100644 libavformat/ffprobedec.c
>>
>> this seems not to apply cleanly
>>
>> can i pick this from some git repo ? (should work better than patch
>> with unavailable ancestors)
>
> Updated, this should apply cleanly on master.

Why we need this hack?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-17 Thread Stefano Sabatini
On date Sunday 2016-09-04 23:25:56 +0200, Michael Niedermayer encoded:
> On Sun, Sep 04, 2016 at 06:24:37PM +0200, Stefano Sabatini wrote:
> > From: Nicolas George 
> > 
> > With several modifications and documentation by Stefano Sabatini
> > .
> > 
> > Signed-off-by: Nicolas George 
> > ---
> >  doc/ffprobe-format.texi  | 130 
> >  libavformat/Makefile |   1 +
> >  libavformat/allformats.c |   2 +
> >  libavformat/ffprobedec.c | 397 
> > +++
> >  4 files changed, 530 insertions(+)
> >  create mode 100644 doc/ffprobe-format.texi
> >  create mode 100644 libavformat/ffprobedec.c
> 
> this seems not to apply cleanly
> 
> can i pick this from some git repo ? (should work better than patch
> with unavailable ancestors)

Updated, this should apply cleanly on master.
-- 
FFmpeg = Fast and Fast Monstrous Power Elected Game
>From bd026e67a4876cff4e50edfb7f0958c87eacbef0 Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Sat, 11 Jan 2014 19:42:41 +0100
Subject: [PATCH] lavf: add ffprobe demuxer

With several modifications and documentation by Stefano Sabatini
.

Signed-off-by: Nicolas George 
---
 doc/ffprobe-format.texi  | 130 
 libavformat/Makefile |   1 +
 libavformat/allformats.c |   1 +
 libavformat/ffprobedec.c | 397 +++
 4 files changed, 529 insertions(+)
 create mode 100644 doc/ffprobe-format.texi
 create mode 100644 libavformat/ffprobedec.c

diff --git a/doc/ffprobe-format.texi b/doc/ffprobe-format.texi
new file mode 100644
index 000..be7af15
--- /dev/null
+++ b/doc/ffprobe-format.texi
@@ -0,0 +1,130 @@
+@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 (FORMAT, STREAM, 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
+@sample{key=value}. In the case of data the key and the value must
+stay on different lines.
+
+Unrecognized values are discarded (this allows the demuxer to accept
+the output generated by @program{ffprobe} without further
+modifications.
+
+The following sections document the fields accepted by each section.
+
+@section FORMAT
+
+@table
+@item nb_streams
+the number of supported streams
+@end table
+
+@section STREAM
+@table
+@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
+@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_tim
+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.
+@end table
+
+@section Examples
+
+A ffprobe demuxer file might look like this:
+@example
+[FORMAT]
+nb_streams=1
+format_name=ffprobe_default
+[/FORMAT]
+[STREAM]
+index=0
+codec_name=timed_id3
+time_base=1/100
+[/STREAM]
+[PACKET]
+stream_index=0
+pts_time=0
+dts_time=0
+duration=N/A
+flags=K
+data=
+f000  0167 42c0 1ed9 81b1 fefc 0440
+57ff d7d1 dfff e11a 3d7e 6cbd  0001
+ce8c 4d9d 108e 25e9 fe
+
+[/PACKET]
+[PACKET]
+stream_index=0
+pts_time=1.00
+duration=N/A
+flags=K
+data=
+f000  0141 9a38 3be5   fffa
+ebc1 3782 7d5e 21e9  abf2 ea4f ed66
+ 0001 ce8c 4d9d 108e 25e9 fe
+
+[/PACKET]
+@end example
+
+The output generated by the @program{ffprobe} default format is valid
+if the following options are also specified:
+@example
+-show_packets -show_data -show_compact_data -show_format -show_streams 

Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-05 Thread Nicolas George
Le decadi 20 fructidor, an CCXXIV, Paul B Mahol a écrit :
> What is "output" in this context?

Imagine you are trying to debug a problem in filters with a particular file.
You suspect an issue with timestamps, or with whatever. To check, you may
want to change the input file slightly. This is not very easy to do right
now. With this patch, ffprobe the input file, then edit the resulting text
file to make your changes with any text editor.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-05 Thread Paul B Mahol
On 9/4/16, Stefano Sabatini  wrote:
> On date Sunday 2016-09-04 18:27:55 +0200, Paul B Mahol encoded:
>> On 9/4/16, Stefano Sabatini  wrote:
>> > From: Nicolas George 
>> >
>> > With several modifications and documentation by Stefano Sabatini
>> > .
>> >
>> > Signed-off-by: Nicolas George 
>> > ---
>> >  doc/ffprobe-format.texi  | 130 
>> >  libavformat/Makefile |   1 +
>> >  libavformat/allformats.c |   2 +
>> >  libavformat/ffprobedec.c | 397
>> > +++
>> >  4 files changed, 530 insertions(+)
>> >  create mode 100644 doc/ffprobe-format.texi
>> >  create mode 100644 libavformat/ffprobedec.c
>> >
>>
>> Why
>
> This was discussed in the thread:
> [PATCH] lavf: add textdata virtual demuxer and demuxer
>
> Basically, my use case is injecting custom data into an output by
> using an ffmpeg commandline.

What is "output" in this context?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-04 Thread Michael Niedermayer
On Sun, Sep 04, 2016 at 06:24:37PM +0200, Stefano Sabatini wrote:
> From: Nicolas George 
> 
> With several modifications and documentation by Stefano Sabatini
> .
> 
> Signed-off-by: Nicolas George 
> ---
>  doc/ffprobe-format.texi  | 130 
>  libavformat/Makefile |   1 +
>  libavformat/allformats.c |   2 +
>  libavformat/ffprobedec.c | 397 
> +++
>  4 files changed, 530 insertions(+)
>  create mode 100644 doc/ffprobe-format.texi
>  create mode 100644 libavformat/ffprobedec.c

this seems not to apply cleanly

can i pick this from some git repo ? (should work better than patch
with unavailable ancestors)


Applying: lavf: add ffprobe demuxer
.git/rebase-apply/patch:16: trailing whitespace.
@chapter ffprobe demuxer format
fatal: sha1 information is lacking or useless (libavformat/Makefile).
error: could not build fake ancestor
Patch failed at 0001 lavf: add ffprobe demuxer
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-04 Thread Stefano Sabatini
On date Sunday 2016-09-04 18:27:55 +0200, Paul B Mahol encoded:
> On 9/4/16, Stefano Sabatini  wrote:
> > From: Nicolas George 
> >
> > With several modifications and documentation by Stefano Sabatini
> > .
> >
> > Signed-off-by: Nicolas George 
> > ---
> >  doc/ffprobe-format.texi  | 130 
> >  libavformat/Makefile |   1 +
> >  libavformat/allformats.c |   2 +
> >  libavformat/ffprobedec.c | 397
> > +++
> >  4 files changed, 530 insertions(+)
> >  create mode 100644 doc/ffprobe-format.texi
> >  create mode 100644 libavformat/ffprobedec.c
> >
> 
> Why

This was discussed in the thread:
[PATCH] lavf: add textdata virtual demuxer and demuxer

Basically, my use case is injecting custom data into an output by
using an ffmpeg commandline.

For specifying data I designed the textdata demuxer, but it was
limited (no support for several streams), so I followed the advice
from Nicolas and adopted the format based on the ffprobe default
output he already developed.

I'll add more documentation and examples at the next iteration.
-- 
FFmpeg = Foolish Fabulous MultiPurpose Erratic Generator
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-04 Thread Paul B Mahol
On 9/4/16, Stefano Sabatini  wrote:
> From: Nicolas George 
>
> With several modifications and documentation by Stefano Sabatini
> .
>
> Signed-off-by: Nicolas George 
> ---
>  doc/ffprobe-format.texi  | 130 
>  libavformat/Makefile |   1 +
>  libavformat/allformats.c |   2 +
>  libavformat/ffprobedec.c | 397
> +++
>  4 files changed, 530 insertions(+)
>  create mode 100644 doc/ffprobe-format.texi
>  create mode 100644 libavformat/ffprobedec.c
>

Why?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] lavf: add ffprobe demuxer

2016-09-04 Thread Stefano Sabatini
From: Nicolas George 

With several modifications and documentation by Stefano Sabatini
.

Signed-off-by: Nicolas George 
---
 doc/ffprobe-format.texi  | 130 
 libavformat/Makefile |   1 +
 libavformat/allformats.c |   2 +
 libavformat/ffprobedec.c | 397 +++
 4 files changed, 530 insertions(+)
 create mode 100644 doc/ffprobe-format.texi
 create mode 100644 libavformat/ffprobedec.c

diff --git a/doc/ffprobe-format.texi b/doc/ffprobe-format.texi
new file mode 100644
index 000..159f4c4
--- /dev/null
+++ b/doc/ffprobe-format.texi
@@ -0,0 +1,130 @@
+@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 (FORMAT, STREAM, PACKET). Each section
+start 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 will 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
+@sample{key=value}. In the case of data the key and the value must
+stay on different lines.
+
+Unrecognized values are discarded (this allows the demuxer to accept
+the output generated by @program{ffprobe} without further
+modifications.
+
+The following sections document the fields accepted by each section.
+
+@section FORMAT
+
+@table
+@item nb_streams
+the number of supported streams
+@end table
+
+@section STREAM
+@table
+@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
+@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_tim
+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
+terminate with an empty line.
+@end table
+
+@section Examples
+
+A ffprobe demuxer file might look like this:
+@example
+[FORMAT]
+nb_streams=1
+format_name=ffprobe_default
+[/FORMAT]
+[STREAM]
+index=0
+codec_name=timed_id3
+time_base=1/100
+[/STREAM]
+[PACKET]
+stream_index=0
+pts_time=0
+dts_time=0
+duration=N/A
+flags=K
+data=
+f000  0167 42c0 1ed9 81b1 fefc 0440
+57ff d7d1 dfff e11a 3d7e 6cbd  0001
+ce8c 4d9d 108e 25e9 fe
+
+[/PACKET]
+[PACKET]
+stream_index=0
+pts_time=1.00
+duration=N/A
+flags=K
+data=
+f000  0141 9a38 3be5   fffa
+ebc1 3782 7d5e 21e9  abf2 ea4f ed66
+ 0001 ce8c 4d9d 108e 25e9 fe
+
+[/PACKET]
+@end example
+
+The output generated by the @program{ffprobe} default format is valid
+if the following options are also specified:
+@example
+-show_packets -show_data -show_compact_data -show_format -show_streams 
-show_packets -show_headers_first
+@end example
+
+Thus, to generate the corresponding output from an input file you can
+do:
+@example
+ffprobe INPUT.ts -show_packets -show_data -show_compact_data -show_format 
-show_streams -show_packets -show_headers_first > OUTPUT.ffprobe
+@end example
+
+@c man end FFPROBE DEMUXER FORMAT
diff --git a/libavformat/Makefile b/libavformat/Makefile
index b68c27c..d222ac2 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_DEFAULT_DEMUXER)   += ffprobedec.o
 OBJS-$(CONFIG_FFTEXTDATA_DEMUXER)+= fftextdatadec.o
 OBJS-$(CONFIG_FFTEXTDATA_MUXER)  += fftextdataenc.o
 OBJS-$(CONFIG_FIFO_MUXER)+= fifo.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index e58e41d..2e10e26 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -123,8 +123,10 @@ void