Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
On Wed, Apr 10, 2024 at 11:08 AM Stefano Sabatini wrote: > On date Sunday 2024-04-07 16:01:27 +0800, Zhao Zhili wrote: > > > > > On Apr 7, 2024, at 14:16, Anton Khirnov wrote: > > > > > > Quoting Andreas Rheinhardt (2024-04-06 13:25:49) > > >> See > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/320849.html > > >> Additionally I do not agree that sorting options by name is the best > > >> way; it should be sorted by what are (believed to be) the most > commonly > > >> used options. > > > > > > +1 > > > > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240106165246.274472-1-stefa...@gmail.com/ > > > > I have the same consideration in another patch. Maybe group related > > options together than sort whole options. > > This hardly works for the aforementioned reasons, no objective > criteria means that there will be never a consistent way to sort the > options. What happens in practice is that options are added more or > less randomly, which doesn't help readability and discoverability > especially when there are many options. > We have TC now. Oh yes, I completely forgot what is TC. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
On date Sunday 2024-04-07 16:01:27 +0800, Zhao Zhili wrote: > > > On Apr 7, 2024, at 14:16, Anton Khirnov wrote: > > > > Quoting Andreas Rheinhardt (2024-04-06 13:25:49) > >> See https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/320849.html > >> Additionally I do not agree that sorting options by name is the best > >> way; it should be sorted by what are (believed to be) the most commonly > >> used options. > > > > +1 > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240106165246.274472-1-stefa...@gmail.com/ > > I have the same consideration in another patch. Maybe group related > options together than sort whole options. This hardly works for the aforementioned reasons, no objective criteria means that there will be never a consistent way to sort the options. What happens in practice is that options are added more or less randomly, which doesn't help readability and discoverability especially when there are many options. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
> On Apr 7, 2024, at 14:16, Anton Khirnov wrote: > > Quoting Andreas Rheinhardt (2024-04-06 13:25:49) >> See https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/320849.html >> Additionally I do not agree that sorting options by name is the best >> way; it should be sorted by what are (believed to be) the most commonly >> used options. > > +1 https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240106165246.274472-1-stefa...@gmail.com/ I have the same consideration in another patch. Maybe group related options together than sort whole options. > > -- > Anton Khirnov > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
Quoting Andreas Rheinhardt (2024-04-06 13:25:49) > See https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/320849.html > Additionally I do not agree that sorting options by name is the best > way; it should be sorted by what are (believed to be) the most commonly > used options. +1 -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
On date Saturday 2024-04-06 13:25:49 +0200, Andreas Rheinhardt wrote: > Stefano Sabatini: > > --- > > libavformat/matroskaenc.c | 12 ++-- > > 1 file changed, 6 insertions(+), 6 deletions(-) > > [...] > See https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/320849.html > Additionally I do not agree that sorting options by name is the best > way; > it should be sorted by what are (believed to be) the most commonly > used options. This is highly arbitrary and subjective, the best way is sort-by-name as already done in other places (and I still find the git blame argument rather weak for the reasons I already explained). The alternative - which I observe all the time - is that when a new option is added an arbitrary place is assigned and more confusion is added. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
Stefano Sabatini:
> ---
> libavformat/matroskaenc.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 566e9f4981..8ebe6e4334 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -3500,20 +3500,20 @@ 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 },
> +{ "allow_raw_vfw", "allow RAW VFW mode", OFFSET(allow_raw_vfw),
> 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 },
> +{ "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 },
> { "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, .unit = "default_mode" },
> +{ "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 },
> { "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, .unit =
> "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, .unit = "default_mode" },
> +{ "live", "Write files assuming it is a live stream.", OFFSET(is_live),
> AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
> { "passthrough", "Use the disposition flag as-is", 0, AV_OPT_TYPE_CONST,
> { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS, .unit = "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 },
> +{ "write_crc32", "write a CRC32 element inside every Level 1 element",
> OFFSET(write_crc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, FLAGS },
> { NULL },
> };
>
See https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/320849.html
Additionally I do not agree that sorting options by name is the best
way; it should be sorted by what are (believed to be) the most commonly
used options.
- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
---
libavformat/matroskaenc.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 566e9f4981..8ebe6e4334 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -3500,20 +3500,20 @@ 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 },
+{ "allow_raw_vfw", "allow RAW VFW mode", OFFSET(allow_raw_vfw),
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 },
+{ "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 },
{ "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, .unit = "default_mode" },
+{ "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 },
{ "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, .unit =
"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, .unit = "default_mode" },
+{ "live", "Write files assuming it is a live stream.", OFFSET(is_live),
AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
{ "passthrough", "Use the disposition flag as-is", 0, AV_OPT_TYPE_CONST, {
.i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS, .unit = "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 },
+{ "write_crc32", "write a CRC32 element inside every Level 1 element",
OFFSET(write_crc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, FLAGS },
{ NULL },
};
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
