[FFmpeg-devel] [PATCH] ffprobe: generalize writer subsection nesting model

[Stefano Sabatini]

On date Wednesday 2012-09-19 00:34:44 +0200, Clément Bœsch encoded:
> On Tue, Sep 18, 2012 at 01:45:33PM +0200, Stefano Sabatini wrote:
> > On date Tuesday 2012-09-18 01:52:14 +0200, Michael Niedermayer encoded:
> > > On Tue, Sep 18, 2012 at 12:43:55AM +0200, Stefano Sabatini wrote:
> > > > On date Tuesday 2012-09-18 00:38:39 +0200, Stefano Sabatini encoded:
> > > > > Discard unflexible structure based on the root/chapter/section structure
> > > > > in favor of a generalized concept of section.
> > > > > 
> > > > > This should allow to represent sections at generic level of nesting, and
> > > > > allow subsections printing selection.
> > > > > 
> > > > > Also, slightly simplify the code.
> > > > > ---
> > > > >  ffprobe.c |  515 +++++++++++++++++++++++++++++--------------------------------
> > > > >  1 files changed, 242 insertions(+), 273 deletions(-)
> > > > 
> > > > Note: I tested the patch against a local fate test for each
> > > > writer. The test file was generated with:
> > > > 
> > > > ffmpeg  -f lavfi -i "aevalsrc=sin(400*PI*2*t)::d=0.5[out0]; testsrc=d=0.5[out1]; testsrc=s=100x100:d=0.5[out2]" \
> > > >    -flags +bitexact  -map 0:0 -map 0:1 -map 0:2 \ 
> > > >    -metadata "title=ffprobe test file" \
> > > >    -metadata "comment='A comment wih XML & JSON special chars': <tag value=\"x\">" \
> 
> Could you put some UTF-8 as well? ♥ or ☭ or whatever…

Done.
 
> > > >    -y tests/data/fate/ffprobe-test.nut
> > > > 
> > > > and I'm not sure if it is best to auto-generate it when fate is run or
> > > > I should just push the generated file to the samples FATE collection.
> > > 
> > > some points to consider:
> > > how large are the files?
> > > how much time is spend by generating them?
> > 
> > > is it worth to test the creation of the files too?
> > 
> > Maybe, but this would also add a dependency on lavfi for testing
> > ffprobe. On the bright side this would allow easy modifications of the
> > generated file (e.g. for adding streams/metadata).
> > 
> > See patch attached (which needs review as I don't have deep knowledge
> > of the fate system).
> > -- 
> > FFmpeg = Fascinating and Fancy Mega Puristic Ephemeral Genius
> 
> > From a1c6594526e821462b2e69b28d1278668dfd3909 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Mon, 17 Sep 2012 21:08:46 +0200
> > Subject: [PATCH] fate: add fate-ffprobe test
> > 
> > ---
> >  tests/Makefile                 |   13 +
> >  tests/fate/ffprobe.mak         |   33 +
> >  tests/ref/fate/ffprobe_compact |  101 +++
> >  tests/ref/fate/ffprobe_csv     |  101 +++
> >  tests/ref/fate/ffprobe_default | 1732 +++++++++++++++++++++++++++++++++++++++
> >  tests/ref/fate/ffprobe_flat    | 1530 +++++++++++++++++++++++++++++++++++
> >  tests/ref/fate/ffprobe_ini     | 1735 ++++++++++++++++++++++++++++++++++++++++
> >  tests/ref/fate/ffprobe_json    | 1625 +++++++++++++++++++++++++++++++++++++
> >  tests/ref/fate/ffprobe_xml     |  113 +++
> >  9 files changed, 6983 insertions(+), 0 deletions(-)
> >  create mode 100644 tests/fate/ffprobe.mak
> >  create mode 100644 tests/ref/fate/ffprobe_compact
> >  create mode 100644 tests/ref/fate/ffprobe_csv
> >  create mode 100644 tests/ref/fate/ffprobe_default
> >  create mode 100644 tests/ref/fate/ffprobe_flat
> >  create mode 100644 tests/ref/fate/ffprobe_ini
> >  create mode 100644 tests/ref/fate/ffprobe_json
> >  create mode 100644 tests/ref/fate/ffprobe_xml
> > 
> > diff --git a/tests/Makefile b/tests/Makefile
> > index 81ce3e1..1e849c3 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -32,6 +32,15 @@ tests/data/vsynth2.yuv: tests/rotozoom$(HOSTEXESUF) | tests/data
> >  
> >  tests/data/%.sw tests/data/asynth% tests/data/vsynth%.yuv tests/vsynth%/00.pgm: TAG = GEN
> >  
> > +FFPROBE_TEST_CMD = ffmpeg$(EXESUF) -f lavfi -i "aevalsrc=sin(400*PI*2*t)::d=0.5[out0]; testsrc=d=0.5[out1]; testsrc=s=100x100:d=0.5[out2]" \
> > +	  -flags +bitexact -map 0:0 -map 0:1 -map 0:2 \
> > +	  -metadata "title=ffprobe test file" \
> > +	  -metadata "comment='A comment wih XML & JSON special chars': <tag value=\"x\">" \
> > +	  -y
> > +
> > +tests/data/ffprobe-test.nut:
> > +	$(M)$(FFPROBE_TEST_CMD) $@
> > +
> >  include $(SRC_PATH)/tests/fate/acodec.mak
> >  include $(SRC_PATH)/tests/fate/vcodec.mak
> >  
> > @@ -53,6 +62,7 @@ include $(SRC_PATH)/tests/fate/dfa.mak
> >  include $(SRC_PATH)/tests/fate/dpcm.mak
> >  include $(SRC_PATH)/tests/fate/ea.mak
> >  include $(SRC_PATH)/tests/fate/filter.mak
> > +include $(SRC_PATH)/tests/fate/ffprobe.mak
> 
> ffprobe before filter if my memory isn't playing tricks with me
> 
> >  include $(SRC_PATH)/tests/fate/flac.mak
> >  include $(SRC_PATH)/tests/fate/fft.mak
> >  include $(SRC_PATH)/tests/fate/h264.mak
> > @@ -103,9 +113,12 @@ FATE_EXTERN += $(FATE_EXTERN-yes)
> >  
> >  FATE += $(FATE-yes)
> >  FATE += $(FATE_LIBAVUTIL)
> > +FATE += $(FATE_FFPROBE)
> >  
> >  $(FATE_FFMPEG) $(FATE_LAVF_FATE) $(FATE_SAMPLES_AVCONV) $(FATE_SAMPLES_FFMPEG): ffmpeg$(EXESUF)
> >  
> > +$(FATE_FFPROBE): ffprobe$(EXESUF)
> > +
> >  $(FATE_LAVF):  $(AREF) $(VREF)
> >  $(FATE_LAVFI): $(VREF) libavfilter/filtfmts-test$(EXESUF)
> >  $(FATE_SEEK):  fate-acodec fate-vsynth2 fate-lavf libavformat/seek-test$(EXESUF)
> > diff --git a/tests/fate/ffprobe.mak b/tests/fate/ffprobe.mak
> > new file mode 100644
> > index 0000000..56f283b
> > --- /dev/null
> > +++ b/tests/fate/ffprobe.mak
> > @@ -0,0 +1,33 @@
> > +FFPROBE_TEST_FILE=tests/data/ffprobe-test.nut
> > +FFPROBE_COMMAND=ffprobe$(EXESUF) -show_streams -show_packets -show_format -show_frames $(FFPROBE_TEST_FILE)
> > +
> > +FATE_FFPROBE += fate-ffprobe_compact
> > +fate-ffprobe_compact: $(FFPROBE_TEST_FILE)
> > +fate-ffprobe_compact: CMD = run $(FFPROBE_COMMAND) -of compact
> > +
> 
> Can't you define a "ffprobefmt" command in fate-run.sh instead?

Why?
 
> > +FATE_FFPROBE += fate-ffprobe_csv
> > +fate-ffprobe_compact: $(FFPROBE_TEST_FILE)
> > +fate-ffprobe_csv: CMD = run $(FFPROBE_COMMAND) -of csv
> > +
> > +FATE_FFPROBE += fate-ffprobe_default
> > +fate-ffprobe_compact: $(FFPROBE_TEST_FILE)
> > +fate-ffprobe_default: CMD = run $(FFPROBE_COMMAND) -of default
> > +
> > +FATE_FFPROBE += fate-ffprobe_flat
> > +fate-ffprobe_compact: $(FFPROBE_TEST_FILE)
> > +fate-ffprobe_flat: CMD = run $(FFPROBE_COMMAND) -of flat
> > +
> > +FATE_FFPROBE += fate-ffprobe_ini
> > +fate-ffprobe_compact: $(FFPROBE_TEST_FILE)
> > +fate-ffprobe_ini: CMD = run $(FFPROBE_COMMAND) -of ini
> > +
> > +FATE_FFPROBE += fate-ffprobe_json
> > +fate-ffprobe_compact: $(FFPROBE_TEST_FILE)
> > +fate-ffprobe_json: CMD = run $(FFPROBE_COMMAND) -of json
> > +
> > +FATE_FFPROBE += fate-ffprobe_xml
> > +fate-ffprobe_compact: $(FFPROBE_TEST_FILE)
> > +fate-ffprobe_xml: CMD = run $(FFPROBE_COMMAND) -of xml
> > +
> > +fate-ffprobe: $(FATE_FFPROBE)
> > +
> 
> [...]
> 
> The references look quite huge... Couldn't you make them contain only 3-4
> frames? (maybe using -frames:v instead of a duration at the generation
> step)

Reduced.

Will post in a dedicated thread.
-- 
FFmpeg = Fundamental Furious Mean Prodigious Emblematic Gladiator