Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc: reformat options table indentation and descriptions

[Marth64 <marth64@proxyid.net>]

Clear and consistent formatting enhances code comprehension and reduces the
likelihood of misinterpretation or errors. As is now, it’s challenging to
read. But I digress

Thanks,

On Mon, Feb 5, 2024 at 06:19 Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Marth64:
> > matroskaenc options table has grown packed over time, and is now
> challenging
> > to read. The purpose of this patch is to reformat the table,
> indentation-wise,
> > and to make the capitalization/endings of each description at least
> consistent.
> >
> > I intend to sort the options in a follow-up patch, but wanted to phase
> > this out to be easier to validate.
> >
> > Signed-off-by: Marth64 <marth64@proxyid.net>
> > ---
> >  libavformat/matroskaenc.c | 42 ++++++++++++++++++++++++++-------------
> >  1 file changed, 28 insertions(+), 14 deletions(-)
> >
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 1457a6890c..2668d1e009 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -3497,20 +3497,34 @@ static const AVCodecTag
> additional_subtitle_tags[] = {
> >  #define OFFSET(x) offsetof(MatroskaMuxContext, x)
> >  #define FLAGS AV_OPT_FLAG_ENCODING_PARAM
> >  static const AVOption options[] = {
> > -    { "reserve_index_space", "Reserve a given amount of space (in
> bytes) at the beginning of the file for the index (cues).",
> OFFSET(reserve_cues_space), AV_OPT_TYPE_INT,   { .i64 = 0 },   0, INT_MAX,
>  FLAGS },
> > -    { "cues_to_front", "Move Cues (the index) to the front by shifting
> data if necessary", OFFSET(move_cues_to_front), AV_OPT_TYPE_BOOL, { .i64 =
> 0}, 0, 1, FLAGS },
> > -    { "cluster_size_limit",  "Store at most the provided amount of
> bytes in a cluster. ",
>  OFFSET(cluster_size_limit), AV_OPT_TYPE_INT  , { .i64 = -1 }, -1,
> INT_MAX,   FLAGS },
> > -    { "cluster_time_limit",  "Store at most the provided number of
> milliseconds in a cluster.",
>  OFFSET(cluster_time_limit), AV_OPT_TYPE_INT64, { .i64 = -1 }, -1,
> INT64_MAX, FLAGS },
> > -    { "dash", "Create a WebM file conforming to WebM DASH
> specification", OFFSET(is_dash), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,
> FLAGS },
> > -    { "dash_track_number", "Track number for the DASH stream",
> OFFSET(dash_track_number), AV_OPT_TYPE_INT, { .i64 = 1 }, 1, INT_MAX, FLAGS
> },
> > -    { "live", "Write files assuming it is a live stream.",
> OFFSET(is_live), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
> > -    { "allow_raw_vfw", "allow RAW VFW mode", OFFSET(allow_raw_vfw),
> AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
> > -    { "flipped_raw_rgb", "Raw RGB bitmaps in VFW mode are stored
> bottom-up", OFFSET(flipped_raw_rgb), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,
> FLAGS },
> > -    { "write_crc32", "write a CRC32 element inside every Level 1
> element", OFFSET(write_crc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, FLAGS },
> > -    { "default_mode", "Controls how a track's FlagDefault is inferred",
> OFFSET(default_mode), AV_OPT_TYPE_INT, { .i64 = DEFAULT_MODE_PASSTHROUGH },
> DEFAULT_MODE_INFER, DEFAULT_MODE_PASSTHROUGH, FLAGS, "default_mode" },
> > -    { "infer", "For each track type, mark each track of disposition
> default as default; if none exists, mark the first track as default.", 0,
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS,
> "default_mode" },
> > -    { "infer_no_subs", "For each track type, mark each track of
> disposition default as default; for audio and video: if none exists, mark
> the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 =
> DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },
> > -    { "passthrough", "Use the disposition flag as-is", 0,
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS,
> "default_mode" },
> > +    { "reserve_index_space", "Reserve a given amount of space (in
> bytes) at the beginning of the file for the index (cues)",
> > +                             OFFSET(reserve_cues_space),
> AV_OPT_TYPE_INT,   { .i64 = 0 },   0,      INT_MAX,    FLAGS },
> > +    { "cues_to_front",       "Move Cues (the index) to the front by
> shifting data if necessary",
> > +                             OFFSET(move_cues_to_front),
> AV_OPT_TYPE_BOOL,  { .i64 = 0 },   0,      1,          FLAGS },
> > +    { "cluster_size_limit",  "Store at most the provided amount of
> bytes in a cluster",
> > +                             OFFSET(cluster_size_limit),
> AV_OPT_TYPE_INT,   { .i64 = -1 },  -1,     INT_MAX,    FLAGS },
> > +    { "cluster_time_limit",  "Store at most the provided number of
> milliseconds in a cluster",
> > +                             OFFSET(cluster_time_limit),
> AV_OPT_TYPE_INT64, { .i64 = -1 },  -1,     INT64_MAX,  FLAGS },
> > +    { "dash",                "Create a WebM file conforming to WebM
> DASH specification",
> > +                             OFFSET(is_dash),
> AV_OPT_TYPE_BOOL,  { .i64 = 0 },   0,      1,          FLAGS },
> > +    { "dash_track_number",   "Track number for the DASH stream",
> > +                             OFFSET(dash_track_number),
> AV_OPT_TYPE_INT,   { .i64 = 1 },   1,      INT_MAX,    FLAGS },
> > +    { "live",                "Write files assuming it is a live stream",
> > +                             OFFSET(is_live),
> AV_OPT_TYPE_BOOL,  { .i64 = 0 },   0,      1,          FLAGS },
> > +    { "allow_raw_vfw",       "Allow RAW VFW mode",
> > +                             OFFSET(allow_raw_vfw),
> AV_OPT_TYPE_BOOL,  { .i64 = 0 },   0,      1,          FLAGS },
> > +    { "flipped_raw_rgb",     "Raw RGB bitmaps in VFW mode are stored
> bottom-up",
> > +                             OFFSET(flipped_raw_rgb),
> AV_OPT_TYPE_BOOL,  { .i64 = 0 },   0,      1,          FLAGS },
> > +    { "write_crc32",         "Write a CRC32 element inside every Level
> 1 element",
> > +                             OFFSET(write_crc),
> AV_OPT_TYPE_BOOL,  { .i64 = 1 },   0,      1,          FLAGS },
> > +    { "default_mode",        "Controls how a track's FlagDefault is
> inferred",
> > +                             OFFSET(default_mode),
>  AV_OPT_TYPE_INT,   { .i64 = DEFAULT_MODE_PASSTHROUGH },
> DEFAULT_MODE_INFER, DEFAULT_MODE_PASSTHROUGH, FLAGS, "default_mode" },
> > +    { "infer",               "For each track type, mark each track of
> disposition default as default; if none exists, mark the first track as
> default",
> > +                             0,
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER },          0, 0, FLAGS,
> "default_mode" },
> > +    { "infer_no_subs",       "For each track type, mark each track of
> disposition default as default; for audio and video: if none exists, mark
> the first track as default",
> > +                             0,
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER_NO_SUBS },  0, 0, FLAGS,
> "default_mode" },
> > +    { "passthrough",         "Use the disposition flag as-is",
> > +                             0,
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH },    0, 0, FLAGS,
> "default_mode" },
> >      { NULL },
> >  };
> >
>
> I do not really see the point of this. It will just interfere with git
> blame when searching for the commit that introduced an option.
>
> - Andreas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".