[FFmpeg-devel] [PATCH] ffprobe: generalize writer subsection nesting model
Stefano Sabatini
stefasab at gmail.com
Wed Sep 19 10:23:14 CEST 2012
On date Tuesday 2012-09-18 23:25:40 +0200, Clément Bœsch encoded:
> On Tue, Sep 18, 2012 at 12:38:39AM +0200, Stefano Sabatini wrote:
> > 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(-)
> >
>
> Good idea :)
>
> > diff --git a/ffprobe.c b/ffprobe.c
> > index a0aee83..d198f90 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -28,6 +28,7 @@
> >
> > #include "libavformat/avformat.h"
> > #include "libavcodec/avcodec.h"
> > +#include "libavutil/avassert.h"
> > #include "libavutil/avstring.h"
> > #include "libavutil/bprint.h"
> > #include "libavutil/opt.h"
> > @@ -66,6 +67,40 @@ static int show_private_data = 1;
> >
> > static char *print_format;
> >
> > +/* section structure definition */
> > +
> > +struct section {
> > + const char *name;
> > + int flags;
> > +};
> > +
> > +#define SECTION_FLAG_IS_WRAPPER 1 ///< the section only contains other sections, but has no internal data
> > +#define SECTION_FLAG_IS_ARRAY 2 ///< the section contains an array of elements of the same type
> > +
> > +struct section root_section = { "root", 1 };
> > +
>
> 1 can be replaced with the flag
>
> > +struct section packets_and_frames_section = { "packets_and_frames", SECTION_FLAG_IS_ARRAY };
> > +
> > +struct section frames_section = { "frames", SECTION_FLAG_IS_ARRAY };
> > +struct section frame_section = { "frame", 0 };
> > +struct section frame_tags_section = { "tags", SECTION_FLAG_IS_ARRAY };
> > +
> > +struct section packets_section = { "packets", SECTION_FLAG_IS_ARRAY };
> > +struct section packet_section = { "packet", 0 };
> > +
> > +struct section error_section = { "error", 0 };
> > +
> > +struct section format_section = { "format", 0 };
> > +struct section format_tags_section = { "tags", SECTION_FLAG_IS_ARRAY };
> > +
> > +struct section library_versions_section = { "library_versions", SECTION_FLAG_IS_ARRAY };
> > +struct section library_version_section = { "library_version", 0 };
> > +struct section program_version_section = { "program_version", 0 };
> > +
> > +struct section streams_section = { "streams", SECTION_FLAG_IS_ARRAY };
> > +struct section stream_section = { "stream", 0 };
> > +struct section stream_tags_section = { "tags", SECTION_FLAG_IS_ARRAY };
> > +
>
> Isn't it possible to do something like:
>
> static const struct section {
> const char *name;
> int flags;
> } section_settings[] = {
> [SECTION_TYPE_ROOT] = {"root", SECTION_FLAG_...},
> [SECTION_TYPE_FRAMES] = {"frames", SECTION_FLAG_...},
> ...
> };
>
> Also, instead of flags, you might want to use bit fields:
> const char *name;
> int wrapper:1;
> int array:1;
> ...
I may need to check on the identity of a section (if parent_section ==
fruits_section), I could still do it embedding the type in the section
type, but looks more verbose, so I see no need for it.
>
> > static const OptionDef *options;
> >
> > /* FFprobe context */
> > @@ -160,13 +195,8 @@ typedef struct Writer {
> > int (*init) (WriterContext *wctx, const char *args, void *opaque);
> > void (*uninit)(WriterContext *wctx);
> >
> > - void (*print_header)(WriterContext *ctx);
> > - void (*print_footer)(WriterContext *ctx);
> > -
> > - void (*print_chapter_header)(WriterContext *wctx, const char *);
> > - void (*print_chapter_footer)(WriterContext *wctx, const char *);
> > - void (*print_section_header)(WriterContext *wctx, const char *);
> > - void (*print_section_footer)(WriterContext *wctx, const char *);
> > + void (*print_section_header)(WriterContext *wctx);
> > + void (*print_section_footer)(WriterContext *wctx);
> > void (*print_integer) (WriterContext *wctx, const char *, long long int);
> > void (*print_rational) (WriterContext *wctx, AVRational *q, char *sep);
> > void (*print_string) (WriterContext *wctx, const char *, const char *);
> > @@ -174,19 +204,24 @@ typedef struct Writer {
> > int flags; ///< a combination or WRITER_FLAG_*
> > } Writer;
> >
> > +#define MAX_SECTION_LEVEL 10
> > +
> > struct WriterContext {
> > const AVClass *class; ///< class of the writer
> > const Writer *writer; ///< the Writer of which this is an instance
> > char *name; ///< name of this writer instance
> > void *priv; ///< private data for use by the filter
> > - unsigned int nb_item; ///< number of the item printed in the given section, starting at 0
> > - unsigned int nb_section; ///< number of the section printed in the given section sequence, starting at 0
> > +
> > unsigned int nb_section_packet; ///< number of the packet section in case we are in "packets_and_frames" section
> > unsigned int nb_section_frame; ///< number of the frame section in case we are in "packets_and_frames" section
> > unsigned int nb_section_packet_frame; ///< nb_section_packet or nb_section_frame according if is_packets_and_frames
> > - unsigned int nb_chapter; ///< number of the chapter, starting at 0
> >
> > - int multiple_sections; ///< tells if the current chapter can contain multiple sections
> > + int level; ///< current level, starting from 0
> > +
> > + ///< number of the item printed in the given section, starting at 0
> > + unsigned int nb_item[MAX_SECTION_LEVEL];
> > + const struct section *sections[MAX_SECTION_LEVEL];
> > +
> > int is_fmt_chapter; ///< tells if the current chapter is "format", required by the print_format_entry option
> > int is_packets_and_frames; ///< tells if the current section is "packets_and_frames"
> > };
> > @@ -234,6 +269,7 @@ static int writer_open(WriterContext **wctx, const Writer *writer,
> >
> > (*wctx)->class = &writer_class;
> > (*wctx)->writer = writer;
> > + (*wctx)->level = -1;
> >
> > if (writer->priv_class) {
> > void *priv_ctx = (*wctx)->priv;
> > @@ -256,64 +292,47 @@ fail:
> > return ret;
> > }
> >
> > -static inline void writer_print_header(WriterContext *wctx)
> > +static inline void writer_print_section_header(WriterContext *wctx,
> > + const struct section *section)
> > {
> > - if (wctx->writer->print_header)
> > - wctx->writer->print_header(wctx);
> > - wctx->nb_chapter = 0;
> > -}
> > + const struct section *parent_section;
> >
> > -static inline void writer_print_footer(WriterContext *wctx)
> > -{
> > - if (wctx->writer->print_footer)
> > - wctx->writer->print_footer(wctx);
> > -}
> > + wctx->level++;
> > + av_assert0(wctx->level <= MAX_SECTION_LEVEL);
> > + parent_section = wctx->level ? wctx->sections[wctx->level-1] : NULL;
> >
> > -static inline void writer_print_chapter_header(WriterContext *wctx,
> > - const char *chapter)
> > -{
> > - wctx->nb_section =
> > - wctx->nb_section_packet = wctx->nb_section_frame =
> > - wctx->nb_section_packet_frame = 0;
> > - wctx->is_packets_and_frames = !strcmp(chapter, "packets_and_frames");
> > - wctx->multiple_sections = !strcmp(chapter, "packets") || !strcmp(chapter, "frames" ) ||
> > - wctx->is_packets_and_frames ||
> > - !strcmp(chapter, "streams") || !strcmp(chapter, "library_versions");
> > - wctx->is_fmt_chapter = !strcmp(chapter, "format");
> > + wctx->nb_item[wctx->level] = 0;
> > + wctx->sections[wctx->level] = section;
>
> According to the assert above, wctx->level can reach MAX_SECTION_LEVEL; an
> overflow looks possible.
Fixed.
>
> >
> > - if (wctx->writer->print_chapter_header)
> > - wctx->writer->print_chapter_header(wctx, chapter);
> > -}
> > + if (section == &packets_and_frames_section) {
> > + wctx->nb_section_packet = wctx->nb_section_frame =
> > + wctx->nb_section_packet_frame = 0;
> > + } else if (parent_section == &packets_and_frames_section) {
> > + wctx->nb_section_packet_frame = section == &packet_section ?
> > + wctx->nb_section_packet : wctx->nb_section_frame;
> > + }
> >
> > -static inline void writer_print_chapter_footer(WriterContext *wctx,
> > - const char *chapter)
> > -{
> > - if (wctx->writer->print_chapter_footer)
> > - wctx->writer->print_chapter_footer(wctx, chapter);
> > - wctx->nb_chapter++;
> > -}
> > + wctx->is_fmt_chapter = !strcmp(section->name, "format");
> >
> > -static inline void writer_print_section_header(WriterContext *wctx,
> > - const char *section)
> > -{
> > - if (wctx->is_packets_and_frames)
> > - wctx->nb_section_packet_frame = !strcmp(section, "packet") ? wctx->nb_section_packet
> > - : wctx->nb_section_frame;
> > if (wctx->writer->print_section_header)
> > - wctx->writer->print_section_header(wctx, section);
> > - wctx->nb_item = 0;
> > + wctx->writer->print_section_header(wctx);
> > }
> >
> > -static inline void writer_print_section_footer(WriterContext *wctx,
> > - const char *section)
> > +static inline void writer_print_section_footer(WriterContext *wctx)
> > {
> > - if (wctx->writer->print_section_footer)
> > - wctx->writer->print_section_footer(wctx, section);
> > - if (wctx->is_packets_and_frames) {
> > - if (!strcmp(section, "packet")) wctx->nb_section_packet++;
> > - else wctx->nb_section_frame++;
> > + const struct section *section = wctx->sections[wctx->level];
> > + const struct section *parent_section = wctx->level ?
> > + wctx->sections[wctx->level-1] : NULL;
> > +
> > + if (parent_section)
> > + wctx->nb_item[wctx->level-1]++;
> > + if (parent_section == &packets_and_frames_section) {
>
> Isn't it possible to use a section->flag for this?
Yes it is, but what do you gain from it?
Todo: yes the show_tags also should be dropped. I'll do it in a
following patch (or integrate in this one if I'll find the time).
--
FFmpeg = Fundamental and Fostering Meaningless Plastic Enhancing Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-ffprobe-generalize-writer-subsection-nesting-model.patch
Type: text/x-diff
Size: 38245 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120919/50ab80ee/attachment.bin>
More information about the ffmpeg-devel
mailing list