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

[Stefano Sabatini]

On date Tuesday 2011-10-18 19:17:32 +0200, Stefano Sabatini encoded:
> On date Monday 2011-10-17 10:15:22 +0200, Tomas Härdin encoded:
> > 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
> 
> Yes, I added the field in a recent commit, updated locally.
>   
> > 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.
> 
> Uhm, maybe I could change it to ffprobeType, need to check which is
> the common convenction for dealing with that.
> 
> > 
> > > +        <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.
> 
> Possible.
> 
> > > +    </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>?
> 
> Yes, but it's convenient to have this information duplicated here, for
> avoiding to check the type of the corresponding stream.
>  
> > > +      <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.
> 
> I'll do.
>  
> > > +      <xsd:attribute name="pts"           type="xsd:integer" use="required" />
> > 
> > Optional xsd:long? The user probably prefers unset timestamps over
> > AV_NOPTS_VALUE.
> 
> That's something which needs some thought.
> 
> Some output formats may prefer to have each field specified
> explicitely, even where there is no available value (e.g. in the
> compact/CSV output, you don't want a variable number of fields per
> section).
> 
> So I suppose we could make this a property of the writer
> (force_missing_fields_display or an equivalent, don't have time to
> find a shortest name now).
> 
> > 
> > > +      <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.
> 
> Yes, this could be changed to a plain int (displayed as 8000).
> 
> I'll try to send an updated version soon.

Updated, I still want to think about the streams/packets in ffprobe
(with no streams/packets container).

Patch depends on the two recently posted ffprobe patches.
-- 
FFmpeg = Frenzy and Foolish Mystic Programmable Eager Game
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-ffprobe-add-XML-writer.patch
Type: text/x-diff
Size: 14855 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111024/fd5af976/attachment.bin>