Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
@ 2024-04-06 11:10 Stefano Sabatini
  2024-04-06 11:25 ` Andreas Rheinhardt
  0 siblings, 1 reply; 7+ messages in thread
From: Stefano Sabatini @ 2024-04-06 11:10 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: 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 },
 };
 
-- 
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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
  2024-04-06 11:10 [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name Stefano Sabatini
@ 2024-04-06 11:25 ` Andreas Rheinhardt
  2024-04-06 11:33   ` Stefano Sabatini
  2024-04-07  6:16   ` Anton Khirnov
  0 siblings, 2 replies; 7+ messages in thread
From: Andreas Rheinhardt @ 2024-04-06 11:25 UTC (permalink / raw)
  To: ffmpeg-devel

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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
  2024-04-06 11:25 ` Andreas Rheinhardt
@ 2024-04-06 11:33   ` Stefano Sabatini
  2024-04-07  6:16   ` Anton Khirnov
  1 sibling, 0 replies; 7+ messages in thread
From: Stefano Sabatini @ 2024-04-06 11:33 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
  2024-04-06 11:25 ` Andreas Rheinhardt
  2024-04-06 11:33   ` Stefano Sabatini
@ 2024-04-07  6:16   ` Anton Khirnov
  2024-04-07  8:01     ` Zhao Zhili
  1 sibling, 1 reply; 7+ messages in thread
From: Anton Khirnov @ 2024-04-07  6:16 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
  2024-04-07  6:16   ` Anton Khirnov
@ 2024-04-07  8:01     ` Zhao Zhili
  2024-04-10  9:07       ` Stefano Sabatini
  0 siblings, 1 reply; 7+ messages in thread
From: Zhao Zhili @ 2024-04-07  8:01 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


> On Apr 7, 2024, at 14:16, Anton Khirnov <anton@khirnov.net> 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-stefasab@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-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".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
  2024-04-07  8:01     ` Zhao Zhili
@ 2024-04-10  9:07       ` Stefano Sabatini
  2024-04-11  9:00         ` Paul B Mahol
  0 siblings, 1 reply; 7+ messages in thread
From: Stefano Sabatini @ 2024-04-10  9:07 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On date Sunday 2024-04-07 16:01:27 +0800, Zhao Zhili wrote:
> 
> > On Apr 7, 2024, at 14:16, Anton Khirnov <anton@khirnov.net> 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-stefasab@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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name
  2024-04-10  9:07       ` Stefano Sabatini
@ 2024-04-11  9:00         ` Paul B Mahol
  0 siblings, 0 replies; 7+ messages in thread
From: Paul B Mahol @ 2024-04-11  9:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Wed, Apr 10, 2024 at 11:08 AM Stefano Sabatini <stefasab@gmail.com>
wrote:

> On date Sunday 2024-04-07 16:01:27 +0800, Zhao Zhili wrote:
> >
> > > On Apr 7, 2024, at 14:16, Anton Khirnov <anton@khirnov.net> 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-stefasab@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-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".

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-04-11  9:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-06 11:10 [FFmpeg-devel] [PATCH] lavf/matroskaenc: sort options by name Stefano Sabatini
2024-04-06 11:25 ` Andreas Rheinhardt
2024-04-06 11:33   ` Stefano Sabatini
2024-04-07  6:16   ` Anton Khirnov
2024-04-07  8:01     ` Zhao Zhili
2024-04-10  9:07       ` Stefano Sabatini
2024-04-11  9:00         ` Paul B Mahol

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git