[FFmpeg-devel] [PATCH 1/1] avformat: Add alignment mode to Pro-MPEG

Moritz Barsnick barsnick at gmx.net
Fri Jan 13 23:30:23 EET 2017


On Fri, Jan 13, 2017 at 09:26:16 -0500, Andreas HÃ¥kon wrote:
>      { "d", "FEC D", OFFSET(d), AV_OPT_TYPE_INT, { .i64 =  5 }, 4, 20, .flags = E },
> +    { "align", "Alignment mode: 0=none; 1=COL (default); 2=ROW", OFFSET(align), AV_OPT_TYPE_INT, { .i64 =  1 }, 0, 2, .flags = E },

This usually calls for an enum. Something like (off the top of my
head):

enum AlignMode {
    ALIGN_MODE_NONE,
    ALIGN_MODE_COL,
    ALIGN_MODE_ROW,
};

    { "align", "alignment mode", OFFSET(align), AV_OPT_TYPE_INT, { .i64 = ALIGN_MODE_COL }, ALIGN_MODE_NONE, ALIGN_MODE_ROW, E, "align" },
        { "none", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = ALIGN_MODE_NONE }, INT_MIN, INT_MAX, E, "align" },
        { "COL",  NULL, 0, AV_OPT_TYPE_CONST, { .i64 = ALIGN_MODE_COL  }, INT_MIN, INT_MAX, E, "align" },
        { "ROW",  NULL, 0, AV_OPT_TYPE_CONST, { .i64 = ALIGN_MODE_ROW  }, INT_MIN, INT_MAX, E, "align" },
[...]

(Or as option enums, call them "col" and "row" perhaps.)

> +        switch (s->align) {
> +        case 0:
case ALIGN_MODE_NONE:

> +        case 2:
case ALIGN_MODE_COL:

> +        // ROW block-aligned
> +        //   un-implemented!

If so, I think you should add a warning and tell the user that the code
is falling back to COL mode.

> +        case 1:
case ALIGN_MODE_ROW:
> +        default:
Can't be reached if you use the option system as above, add a warning
perhaps.

Moritz


More information about the ffmpeg-devel mailing list