[FFmpeg-devel] [PATCH] ffprobe integration

[Michael Niedermayer]

On Sun, Feb 07, 2010 at 03:55:24PM +0100, Stefano Sabatini wrote:
[...]
> +const Unit measure_units[UNIT_NB] = {
> +    [UNIT_NONE]            = { UNIT_NONE           , ""       },
> +    [UNIT_SECOND]          = { UNIT_SECOND         , "s"      },
> +    [UNIT_HERTZ]           = { UNIT_HERTZ          , "Hz"     },
> +    [UNIT_BIT]             = { UNIT_BIT            , "bit"    },
> +    [UNIT_BYTE]            = { UNIT_BYTE           , "byte"   },
> +    [UNIT_BYTE_PER_SECOND] = { UNIT_BYTE_PER_SECOND, "byte/s" },
> +    [UNIT_BIT_PER_SECOND]  = { UNIT_BIT_PER_SECOND , "bit/s"  }
> +};

as said this indirection over the table is silly
please use "byte" "bit" "Hz" ... dont invent a complicated system to
obfuscate constant strings


> +
> +/** Print unit of measure. */
> +#define VALUE_STRING_USE_UNIT                      0x0001
> +
> +/** Print SI (binary or decimal) prefixes. */
> +#define VALUE_STRING_USE_PREFIX                    0x0002
> +
> +/** Use binary type prefixes, implies the use of prefixes */
> +#define VALUE_STRING_USE_BINARY_PREFIX             0x0004
> +
> +/** Force the use of binary prefixes with bytes measure units. */
> +#define VALUE_STRING_USE_BYTE_BINARY_PREFIX        0x0008
> +
> +/* Use sexagesimal time format HH:MM:SS.MICROSECONDS. */
> +#define VALUE_STRING_USE_SEXAGESIMAL_TIME_FORMAT   0x0010
> +
> +struct flag_descriptor value_string_flag_descriptors[] = {
> +    { "unit"              ,  VALUE_STRING_USE_UNIT                   },
> +    { "prefix"            , VALUE_STRING_USE_PREFIX                  },
> +    { "byte_binary_prefix", VALUE_STRING_USE_BYTE_BINARY_PREFIX      },
> +    { "sexagesimal"       , VALUE_STRING_USE_SEXAGESIMAL_TIME_FORMAT },
> +    { "pretty"            , VALUE_STRING_USE_UNIT                    |
> +                            VALUE_STRING_USE_PREFIX                  |
> +                            VALUE_STRING_USE_BYTE_BINARY_PREFIX      |
> +                            VALUE_STRING_USE_SEXAGESIMAL_TIME_FORMAT },
> +    { NULL }
> +};
> +
> +static int opt_format(const char *opt, const char *arg)
> +{
> +    return get_flags(&value_string_flags, value_string_flag_descriptors, arg);
> +}

please use
static int print_unit;
static int use_prefix;
...

this is MUCH simpler



[...]

> +
> +static void show_stream(AVFormatContext *fmt_ctx, int stream_idx)
> +{
> +    AVStream *stream = fmt_ctx->streams[stream_idx];
> +    AVCodecContext *dec_ctx;
> +    AVCodec *dec;
> +    char val_str[128];
> +    AVMetadataTag *tag;
> +    char a, b, c, d;
> +

> +    printf("[STREAM]\n");
> +
> +    printf("index=%d\n",        stream->index);

can be merged, and this applies to more printfs

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100207/93c8fb63/attachment.pgp>