diff mbox series

[FFmpeg-devel,1/2] libsvt-av1: Consistently use - rather than _

Message ID 20200729225008.1893933-1-sw@jkqxz.net
State New
Headers show
Series [FFmpeg-devel,1/2] libsvt-av1: Consistently use - rather than _
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Mark Thompson July 29, 2020, 10:50 p.m. UTC
Matching libaom-av1.
---
 libavcodec/libsvt_av1.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

James Almer July 30, 2020, 2:53 a.m. UTC | #1
On 7/29/2020 7:50 PM, Mark Thompson wrote:
> Matching libaom-av1.
> ---
>  libavcodec/libsvt_av1.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/libsvt_av1.c b/libavcodec/libsvt_av1.c
> index c7ae5f9691..3229551d2b 100644
> --- a/libavcodec/libsvt_av1.c
> +++ b/libavcodec/libsvt_av1.c
> @@ -483,7 +483,7 @@ static const AVOption options[] = {
>          { "3level", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 3 },  INT_MIN, INT_MAX, VE, "hielevel" },
>          { "4level", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 },  INT_MIN, INT_MAX, VE, "hielevel" },
>  
> -    { "la_depth", "Look ahead distance [0, 120]", OFFSET(la_depth),
> +    { "la-depth", "Look ahead distance [0, 120]", OFFSET(la_depth),
>        AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 120, VE },
>  
>      { "preset", "Encoding preset [0, 8]",
> @@ -533,7 +533,7 @@ static const AVOption options[] = {
>      { "qp", "QP value for intra frames", OFFSET(qp),
>        AV_OPT_TYPE_INT, { .i64 = 50 }, 0, 63, VE },
>  
> -    { "sc_detection", "Scene change detection", OFFSET(scd),
> +    { "sc-detection", "Scene change detection", OFFSET(scd),
>        AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
>  
>      { "tile-columns", "Log2 of number of tile columns to use", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VE},
> @@ -543,7 +543,7 @@ static const AVOption options[] = {
>  };
>  
>  static const AVClass class = {
> -    .class_name = "libsvt_av1",
> +    .class_name = "libsvt-av1",
>      .item_name  = av_default_item_name,
>      .option     = options,
>      .version    = LIBAVUTIL_VERSION_INT,
> @@ -558,7 +558,7 @@ static const AVCodecDefault eb_enc_defaults[] = {
>  };
>  
>  AVCodec ff_libsvt_av1_encoder = {
> -    .name           = "libsvt_av1",
> +    .name           = "libsvt-av1",
>      .long_name      = NULL_IF_CONFIG_SMALL("SVT-AV1(Scalable Video Technology for AV1) encoder"),
>      .priv_data_size = sizeof(SvtContext),
>      .type           = AVMEDIA_TYPE_VIDEO,
> @@ -573,5 +573,5 @@ AVCodec ff_libsvt_av1_encoder = {
>      .priv_class     = &class,
>      .defaults       = eb_enc_defaults,
>      .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> -    .wrapper_name   = "libsvt_av1",
> +    .wrapper_name   = "libsvtav1",
>  };

The name could also be simply libsvtav1, same as the library (and the
configure option), since unlike libvpx and (in theory) libaom, no other
codec will be supported by it. HEVC is in a different library.

LGTM either way.
Nicolas George July 30, 2020, 8:05 a.m. UTC | #2
Mark Thompson (12020-07-29):
> Matching libaom-av1.

As for options, most of FFmpeg uses underscores. It would be better to
use them too here.

Also, you cannot rename options, you have to create aliases and
deprecate the original. Otherwise you break user cases.

Regards,
James Almer July 30, 2020, 1:15 p.m. UTC | #3
On 7/30/2020 5:05 AM, Nicolas George wrote:
> Mark Thompson (12020-07-29):
>> Matching libaom-av1.
> 
> As for options, most of FFmpeg uses underscores. It would be better to
> use them too here.
> 
> Also, you cannot rename options, you have to create aliases and
> deprecate the original. Otherwise you break user cases.

This encoder was pushed yesterday night, which is why this change is
being suggested. It's not going to break anyone's use cases.

> 
> Regards,
> 
> 
> _______________________________________________
> 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".
>
Nicolas George July 30, 2020, 1:23 p.m. UTC | #4
James Almer (12020-07-30):
> This encoder was pushed yesterday night, which is why this change is
> being suggested. It's not going to break anyone's use cases.

Yes, I realized that afterwards and forgot to amend.

But options with underscores are still the preferred form in the
project.

Regards,
Mark Thompson July 30, 2020, 9:31 p.m. UTC | #5
On 30/07/2020 03:53, James Almer wrote:
 > The name could also be simply libsvtav1, same as the library (and the
 > configure option), since unlike libvpx and (in theory) libaom, no other
 > codec will be supported by it. HEVC is in a different library.

That makes more sense.  I'll use libsvtav1 everywhere.

On 30/07/2020 14:23, Nicolas George wrote:> But options with underscores are still the preferred form in the
> project.
... and we are never going to achieve consistency by inheriting options with dashes in the name of backward compatibility.  Fair.

New version to follow.

Thanks,

- Mark
diff mbox series

Patch

diff --git a/libavcodec/libsvt_av1.c b/libavcodec/libsvt_av1.c
index c7ae5f9691..3229551d2b 100644
--- a/libavcodec/libsvt_av1.c
+++ b/libavcodec/libsvt_av1.c
@@ -483,7 +483,7 @@  static const AVOption options[] = {
         { "3level", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 3 },  INT_MIN, INT_MAX, VE, "hielevel" },
         { "4level", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 4 },  INT_MIN, INT_MAX, VE, "hielevel" },
 
-    { "la_depth", "Look ahead distance [0, 120]", OFFSET(la_depth),
+    { "la-depth", "Look ahead distance [0, 120]", OFFSET(la_depth),
       AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 120, VE },
 
     { "preset", "Encoding preset [0, 8]",
@@ -533,7 +533,7 @@  static const AVOption options[] = {
     { "qp", "QP value for intra frames", OFFSET(qp),
       AV_OPT_TYPE_INT, { .i64 = 50 }, 0, 63, VE },
 
-    { "sc_detection", "Scene change detection", OFFSET(scd),
+    { "sc-detection", "Scene change detection", OFFSET(scd),
       AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
 
     { "tile-columns", "Log2 of number of tile columns to use", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VE},
@@ -543,7 +543,7 @@  static const AVOption options[] = {
 };
 
 static const AVClass class = {
-    .class_name = "libsvt_av1",
+    .class_name = "libsvt-av1",
     .item_name  = av_default_item_name,
     .option     = options,
     .version    = LIBAVUTIL_VERSION_INT,
@@ -558,7 +558,7 @@  static const AVCodecDefault eb_enc_defaults[] = {
 };
 
 AVCodec ff_libsvt_av1_encoder = {
-    .name           = "libsvt_av1",
+    .name           = "libsvt-av1",
     .long_name      = NULL_IF_CONFIG_SMALL("SVT-AV1(Scalable Video Technology for AV1) encoder"),
     .priv_data_size = sizeof(SvtContext),
     .type           = AVMEDIA_TYPE_VIDEO,
@@ -573,5 +573,5 @@  AVCodec ff_libsvt_av1_encoder = {
     .priv_class     = &class,
     .defaults       = eb_enc_defaults,
     .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
-    .wrapper_name   = "libsvt_av1",
+    .wrapper_name   = "libsvtav1",
 };