[FFmpeg-devel] [PATCH] ffprobe: add XML output

[Tomas Härdin]

On Wed, 2011-10-12 at 16:06 +0200, Stefano Sabatini wrote:
> On date Sunday 2011-10-09 13:42:38 +0200, Stefano Sabatini encoded:
> > ---
> >  Makefile         |    2 +-
> >  doc/ffprobe.texi |   23 +++++++
> >  doc/ffprobe.xsd  |   90 ++++++++++++++++++++++++++

Extra brownie points for having a schema :)
It seems to be missing an entry for sample_fmt:

$ xmllint --schema ffprobe.xsd ffprobe.xml
ffprobe.xml:123: element stream: Schemas validity error : Element 'stream', attribute 'sample_fmt': The attribute 'sample_fmt' is not allowed.
ffprobe.xml fails to validate


Schema feedback follows. Some of it may require code changes, such as preferring optional over "N/A":

> +    <xsd:element name="ffprobe" type="ffprobe:ffprobe"/>
> +
> +    <xsd:complexType name="ffprobe">

I'm not particularly fond of having a complexType and an element with
the same name. It may cause problems with some tools I think.

> +        <xsd:sequence>
> +            <xsd:element name="packets"  type="ffprobe:packets" minOccurs="0" maxOccurs="1" />
> +            <xsd:element name="streams"  type="ffprobe:streams" minOccurs="0" maxOccurs="1" />
> +            <xsd:element name="format"   type="ffprobe:format"  minOccurs="0" maxOccurs="1" />
> +        </xsd:sequence>

IMO just having

    <xsd:element name="packet"  type="ffprobe:packet" minOccurs="0" maxOccurs="unbounded" />

is enough (and similarly for the others). As in just using straight-up
element arrays without wrapping them in an extra level of elements.
Would require the code to be changed though, and I don't consider it
that big of a problem.

> +    </xsd:complexType>
> +
> +    <xsd:complexType name="packets">
> +        <xsd:sequence>
> +            <xsd:element name="packet" type="ffprobe:packet" minOccurs="0" maxOccurs="unbounded"/>
> +        </xsd:sequence>
> +    </xsd:complexType>
> +
> +    <xsd:complexType name="packet">
> +      <xsd:attribute name="codec_type"    type="xsd:string"  use="required" />

Isn't codec_type implied from <streams>?

> +      <xsd:attribute name="stream_index"  type="xsd:integer" use="required" />

Let's make the Java people happy and use xsd:int instead of xsd:integer
wherever possible - JAXB turns xsd:integer into BigInteger.

> +      <xsd:attribute name="pts"           type="xsd:integer" use="required" />

Optional xsd:long? The user probably prefers unset timestamps over
AV_NOPTS_VALUE.

> +      <xsd:attribute name="pts_time"      type="xsd:decimal" use="required" />

xsd:decimal is non-scientific. The code seems to output in scientific
notation for large number AFAICT, so xsd:float is appropriate here and
elsewhere.
Optional as well I suppose, instead of outputing "N/A".

> +      <xsd:attribute name="dts"           type="xsd:integer" use="required" />

Optional xsd:long

> +      <xsd:attribute name="dts_time"      type="xsd:decimal" use="required" />

Optional xsd:float

> +      <xsd:attribute name="duration"      type="xsd:integer" use="required" />

xsd:int. Maybe optional, but I think the user may understand 0 == unknown

> +      <xsd:attribute name="duration_time" type="xsd:decimal" use="required" />

xsd:float

> +      <xsd:attribute name="size"          type="xsd:decimal" use="required" />

xsd:int since xsd:decimal allows decimals

> +      <xsd:attribute name="pos"           type="xsd:integer" use="required" />

xsd:long

> +      <xsd:attribute name="flags"         type="xsd:string"  use="required" />
> +    </xsd:complexType>
> +
> +    <xsd:complexType name="streams">
> +        <xsd:sequence>
> +            <xsd:element name="stream" type="ffprobe:stream" minOccurs="0" maxOccurs="unbounded"/>
> +        </xsd:sequence>
> +    </xsd:complexType>
> +
> +    <xsd:complexType name="stream">
> +      <xsd:attribute name="index"            type="xsd:integer" use="required"/>

xsd:int

> +      <xsd:attribute name="codec_name"       type="xsd:string"  use="required"/>
> +      <xsd:attribute name="codec_long_name"  type="xsd:string"  use="required"/>
> +      <xsd:attribute name="codec_type"       type="xsd:string"  use="required"/>
> +      <xsd:attribute name="codec_time_base"  type="xsd:string"  use="required"/>
> +      <xsd:attribute name="codec_tag"        type="xsd:string"  use="required"/>
> +      <xsd:attribute name="codec_tag_string" type="xsd:string"  use="required"/>
> +
> +      <!-- audio attributes -->
> +      <xsd:attribute name="sample_rate"      type="xsd:string" />

Uhm, xsd:int? The example XML has sample_rate="8000.000000" though, for
which xsd:float fits, but stinks.

> +      <xsd:attribute name="channels"         type="xsd:integer"/>
> +      <xsd:attribute name="bits_per_sample"  type="xsd:integer"/>
> +
> +      <!-- video attributes -->
> +      <xsd:attribute name="width"        type="xsd:integer"/>
> +      <xsd:attribute name="height"       type="xsd:integer"/>
> +      <xsd:attribute name="has_b_frames" type="xsd:integer"/>
> +      <xsd:attribute name="pix_fmt"      type="xsd:string" />
> +      <xsd:attribute name="level"        type="xsd:integer"/>

s/xsd:integer/xsd:int/g

> +      <xsd:attribute name="r_frame_rate"     type="xsd:string"  use="required"/>
> +      <xsd:attribute name="avg_frame_rate"   type="xsd:string"  use="required"/>
> +      <xsd:attribute name="time_base"        type="xsd:string"  use="required"/>
> +      <xsd:attribute name="start_time"       type="xsd:decimal" use="required"/>

Optional xsd:float

> +      <xsd:attribute name="duration"         type="xsd:string"  use="required"/>

Optional xsd:float

> +    </xsd:complexType>
> +
> +    <xsd:complexType name="format">
> +      <xsd:sequence>
> +        <xsd:element name="tag" type="ffprobe:tag" minOccurs="0" maxOccurs="unbounded"/>
> +      </xsd:sequence>
> +
> +      <xsd:attribute name="filename"         type="xsd:string"  use="required"/>
> +      <xsd:attribute name="nb_streams"       type="xsd:integer" use="required"/>

Superfluous? Code outputs it automagically I suppose, so whatever.

> +      <xsd:attribute name="format_name"      type="xsd:string"  use="required"/>
> +      <xsd:attribute name="format_long_name" type="xsd:string"  use="required"/>
> +      <xsd:attribute name="start_time"       type="xsd:decimal" use="required"/>

Optional xsd:float

> +      <xsd:attribute name="duration"         type="xsd:string"  use="required"/>

Optional xsd:float

> +      <xsd:attribute name="size"             type="xsd:decimal" use="required"/>

xsd:long

> +      <xsd:attribute name="bit_rate"         type="xsd:string"  use="required"/>

xsd:float

/Tomas