[FFmpeg-devel] [PATCH] ffprobe: add/extend disposition printing support
Clément Bœsch
ubitux at gmail.com
Sat Sep 29 00:12:01 CEST 2012
On Fri, Sep 28, 2012 at 07:32:07PM +0200, Stefano Sabatini wrote:
> This generalizes the previous work on disposition printing.
>
> Disposition flags are shown in a dedicated section, which should improve
> output intellegibility, extensibility and filtering operations.
>
\o/
> This breaks output syntax for the recently introduced disposition
> printing.
> ---
> doc/ffprobe.xsd | 18 +++++++++++--
> ffprobe.c | 28 ++++++++++++++++----
> tests/ref/fate/ffprobe_compact | 6 ++--
> tests/ref/fate/ffprobe_csv | 6 ++--
> tests/ref/fate/ffprobe_default | 41 +++++++++++++++++++++++++------
> tests/ref/fate/ffprobe_flat | 41 +++++++++++++++++++++++++------
> tests/ref/fate/ffprobe_ini | 47 +++++++++++++++++++++++++++++------
> tests/ref/fate/ffprobe_json | 53 +++++++++++++++++++++++++++++++--------
> tests/ref/fate/ffprobe_xml | 12 +++++++--
> 9 files changed, 199 insertions(+), 53 deletions(-)
>
> diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd
> index 403d59e..193c537 100644
> --- a/doc/ffprobe.xsd
> +++ b/doc/ffprobe.xsd
> @@ -86,9 +86,24 @@
> </xsd:sequence>
> </xsd:complexType>
>
> + <xsd:complexType name="streamDispositionType">
> + <xsd:attribute name="default" type="xsd:int" use="required" />
> + <xsd:attribute name="dub" type="xsd:int" use="required" />
> + <xsd:attribute name="original" type="xsd:int" use="required" />
> + <xsd:attribute name="comment" type="xsd:int" use="required" />
> + <xsd:attribute name="lyrics" type="xsd:int" use="required" />
> + <xsd:attribute name="karaoke" type="xsd:int" use="required" />
> + <xsd:attribute name="forced" type="xsd:int" use="required" />
> + <xsd:attribute name="hearing_impaired" type="xsd:int" use="required" />
> + <xsd:attribute name="visual_impaired" type="xsd:int" use="required" />
> + <xsd:attribute name="clean_effects" type="xsd:int" use="required" />
> + <xsd:attribute name="attached_pic" type="xsd:int" use="required" />
> + </xsd:complexType>
> +
> <xsd:complexType name="streamType">
> <xsd:sequence>
> <xsd:element name="tag" type="ffprobe:tagType" minOccurs="0" maxOccurs="unbounded"/>
> + <xsd:element name="disposition" type="ffprobe:streamDispositionType" minOccurs="0" maxOccurs="1"/>
> </xsd:sequence>
>
> <xsd:attribute name="index" type="xsd:int" use="required"/>
> @@ -100,8 +115,6 @@
> <xsd:attribute name="codec_tag" type="xsd:string" use="required"/>
> <xsd:attribute name="codec_tag_string" type="xsd:string" use="required"/>
> <xsd:attribute name="extradata" type="xsd:string" />
> - <xsd:attribute name="default" type="xsd:int" use="required"/>
> - <xsd:attribute name="forced" type="xsd:int" use="required"/>
>
> <!-- video attributes -->
> <xsd:attribute name="width" type="xsd:int"/>
> @@ -112,7 +125,6 @@
> <xsd:attribute name="pix_fmt" type="xsd:string"/>
> <xsd:attribute name="level" type="xsd:int"/>
> <xsd:attribute name="timecode" type="xsd:string"/>
> - <xsd:attribute name="attached_pic" type="xsd:int"/>
>
> <!-- audio attributes -->
> <xsd:attribute name="sample_fmt" type="xsd:string"/>
> diff --git a/ffprobe.c b/ffprobe.c
> index fc7e9ff..217aefd 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -99,6 +99,7 @@ typedef enum {
> SECTION_ID_PROGRAM_VERSION,
> SECTION_ID_ROOT,
> SECTION_ID_STREAM,
> + SECTION_ID_STREAM_DISPOSITION,
> SECTION_ID_STREAMS,
> SECTION_ID_STREAM_TAGS
> } SectionID;
> @@ -118,6 +119,7 @@ static const struct section sections[] = {
> [SECTION_ID_PROGRAM_VERSION] = { SECTION_ID_PROGRAM_VERSION, "program_version", 0 },
> [SECTION_ID_ROOT] = { SECTION_ID_ROOT, "root", SECTION_FLAG_IS_WRAPPER },
> [SECTION_ID_STREAM] = { SECTION_ID_STREAM, "stream", 0 },
> + [SECTION_ID_STREAM_DISPOSITION] = { SECTION_ID_STREAM_DISPOSITION, "disposition", 0 },
Why not use the SECTION_FLAG_HAS_VARIABLE_FIELDS/.element_name thing?
Wouldn't it break some outputs if we add more flags?
> [SECTION_ID_STREAMS] = { SECTION_ID_STREAMS, "streams", SECTION_FLAG_IS_ARRAY },
> [SECTION_ID_STREAM_TAGS] = { SECTION_ID_STREAM_TAGS, "tags", SECTION_FLAG_HAS_VARIABLE_FIELDS, .element_name = "tag" },
> };
> @@ -1681,10 +1683,6 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> print_str("codec_tag_string", val_str);
> print_fmt("codec_tag", "0x%04x", dec_ctx->codec_tag);
>
> - /* Print useful disposition */
> - print_int("default", !!(stream->disposition & AV_DISPOSITION_DEFAULT));
> - print_int("forced", !!(stream->disposition & AV_DISPOSITION_FORCED));
> -
> switch (dec_ctx->codec_type) {
> case AVMEDIA_TYPE_VIDEO:
> print_int("width", dec_ctx->width);
> @@ -1713,8 +1711,6 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> } else {
> print_str_opt("timecode", "N/A");
> }
> - print_int("attached_pic",
> - !!(stream->disposition & AV_DISPOSITION_ATTACHED_PIC));
> break;
>
> case AVMEDIA_TYPE_AUDIO:
> @@ -1761,6 +1757,26 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> if (do_show_data)
> writer_print_data(w, "extradata", dec_ctx->extradata,
> dec_ctx->extradata_size);
> +
> + /* Print disposition information */
> + writer_print_section_header(w, SECTION_ID_STREAM_DISPOSITION);
> +#define PRINT_DISPOSITION(flagname, name) do { \
> + print_int(name, !!(stream->disposition & AV_DISPOSITION_##flagname)); \
nit: weird '\' positioning
> + } while (0)
> +
> + PRINT_DISPOSITION(DEFAULT, "default");
> + PRINT_DISPOSITION(DUB, "dub");
> + PRINT_DISPOSITION(ORIGINAL, "original");
> + PRINT_DISPOSITION(COMMENT, "comment");
> + PRINT_DISPOSITION(LYRICS, "lyrics");
> + PRINT_DISPOSITION(KARAOKE, "karaoke");
> + PRINT_DISPOSITION(FORCED, "forced");
> + PRINT_DISPOSITION(HEARING_IMPAIRED, "hearing_impaired");
> + PRINT_DISPOSITION(VISUAL_IMPAIRED, "visual_impaired");
> + PRINT_DISPOSITION(CLEAN_EFFECTS, "clean_effects");
> + PRINT_DISPOSITION(ATTACHED_PIC, "attached_pic");
> + writer_print_section_footer(w);
> +
Would it make sense to use WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS to avoid
printing a lot of zero entries when unnecessary?
[...]
> filename=tests/data/ffprobe-test.nut
> diff --git a/tests/ref/fate/ffprobe_flat b/tests/ref/fate/ffprobe_flat
> index e9e2f87..5a0b6d8 100644
> --- a/tests/ref/fate/ffprobe_flat
> +++ b/tests/ref/fate/ffprobe_flat
> @@ -425,8 +425,6 @@ streams.stream.0.codec_type="audio"
> streams.stream.0.codec_time_base="1/44100"
> streams.stream.0.codec_tag_string="[1][0][0][0]"
> streams.stream.0.codec_tag="0x0001"
> -streams.stream.0.default=0
> -streams.stream.0.forced=0
> streams.stream.0.sample_fmt="s16"
> streams.stream.0.sample_rate="44100"
> streams.stream.0.channels=1
> @@ -443,6 +441,17 @@ streams.stream.0.bit_rate="705600"
> streams.stream.0.nb_frames="N/A"
> streams.stream.0.nb_read_frames="6"
> streams.stream.0.nb_read_packets="6"
> +streams.stream.0.disposition.default=0
> +streams.stream.0.disposition.dub=0
> +streams.stream.0.disposition.original=0
> +streams.stream.0.disposition.comment=0
> +streams.stream.0.disposition.lyrics=0
> +streams.stream.0.disposition.karaoke=0
> +streams.stream.0.disposition.forced=0
> +streams.stream.0.disposition.hearing_impaired=0
> +streams.stream.0.disposition.visual_impaired=0
> +streams.stream.0.disposition.clean_effects=0
> +streams.stream.0.disposition.attached_pic=0
Sexy :)
[...]
Looks great, thanks.
--
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120929/eb22f963/attachment.asc>
More information about the ffmpeg-devel
mailing list