Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-06 Thread Lou Logan
On Tue, Dec 5, 2017, at 10:23 PM, Lou Logan wrote:
> As it is late here I plan to test and commit this tomorrow unless
> someone else does first.

Pushed. Thanks for the patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Lou Logan
On Tue, Dec 5, 2017, at 09:36 PM, Gyan Doshi wrote:
> Both done.

Thanks, and LGTM.

As it is late here I plan to test and commit this tomorrow unless
someone else does first.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Gyan Doshi


On 12/6/2017 4:08 AM, Lou Logan wrote:


Nit: change "set :" to "set:".

Would it be possible to list the profiles with x265_profile_names
similar to what is done for libx264?


Both done.
From 607ed6fd623cee0e2d49a70d9446b8360c799498 Mon Sep 17 00:00:00 2001
From: Gyan Doshi 
Date: Tue, 5 Dec 2017 13:17:53 +0530
Subject: [PATCH] avcodec/libx265 - Add named option to set profile

Adds call to x265_param_apply_profile after x265_param_parse.
Added as private option since HEVC profiles other than
Main, Main 10 and MSP in AVCodecContext are consolidated in a single
constant.
---
 libavcodec/libx265.c | 14 ++
 libavcodec/version.h |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 4456e300f2..cbb106aeed 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -45,6 +45,7 @@ typedef struct libx265Context {
 int   forced_idr;
 char *preset;
 char *tune;
+char *profile;
 char *x265_opts;
 } libx265Context;
 
@@ -220,6 +221,18 @@ static av_cold int libx265_encode_init(AVCodecContext 
*avctx)
 }
 }
 
+if (ctx->profile) {
+if (ctx->api->param_apply_profile(ctx->params, ctx->profile) < 0) {
+int i;
+av_log(avctx, AV_LOG_ERROR, "Invalid or incompatible profile set: 
%s.\n", ctx->profile);
+av_log(avctx, AV_LOG_INFO, "Possible profiles:");
+for (i = 0; x265_profile_names[i]; i++)
+av_log(avctx, AV_LOG_INFO, " %s", x265_profile_names[i]);
+av_log(avctx, AV_LOG_INFO, "\n");
+return AVERROR(EINVAL);
+}
+}
+
 ctx->encoder = ctx->api->encoder_open(ctx->params);
 if (!ctx->encoder) {
 av_log(avctx, AV_LOG_ERROR, "Cannot open libx265 encoder.\n");
@@ -392,6 +405,7 @@ static const AVOption options[] = {
 { "forced-idr",  "if forcing keyframes, force them as IDR frames", 
 OFFSET(forced_idr),AV_OPT_TYPE_BOOL,   { .i64 =  0 },  0,  
 1, VE },
 { "preset",  "set the x265 preset",
 OFFSET(preset),AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { "tune","set the x265 tune parameter",
 OFFSET(tune),  AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
+{ "profile", "set the x265 profile",   
 OFFSET(profile),   AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { "x265-params", "set the x265 configuration using a :-separated list of 
key=value parameters", OFFSET(x265_opts), AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { NULL }
 };
diff --git a/libavcodec/version.h b/libavcodec/version.h
index d67b689142..3b5c3000be 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@
 
 #define LIBAVCODEC_VERSION_MAJOR  58
 #define LIBAVCODEC_VERSION_MINOR   6
-#define LIBAVCODEC_VERSION_MICRO 102
+#define LIBAVCODEC_VERSION_MICRO 103
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
LIBAVCODEC_VERSION_MINOR, \
-- 
2.11.1.windows.1___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Lou Logan
On Tue, Dec 5, 2017, at 03:28 PM, Bang He wrote:
> can it list profiles and levels?

I don't know (also, too lazy to look into it). That's why I asked.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Bang He
can it list profiles and levels?

On Wed, Dec 6, 2017 at 6:38 AM, Lou Logan  wrote:

> On Tue, 5 Dec 2017 19:46:09 +0530
> Gyan Doshi  wrote:
>
> > From 13ad80871978fe7e5837863e0e2f7b7d6b356155 Mon Sep 17 00:00:00 2001
> > From: Gyan Doshi 
> > Date: Tue, 5 Dec 2017 13:17:53 +0530
> > Subject: [PATCH] avcodec/libx265 - Add named option to set profile
> >
> > Adds call to x265_param_apply_profile after x265_param_parse.
> > Added as private option since HEVC profiles other than
> > Main, Main 10 and MSP in AVCodecContext are consolidated in a single
> > constant.
> > ---
> >  libavcodec/libx265.c | 9 +
> >  libavcodec/version.h | 2 +-
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> > index 4456e300f2..4058deac1c 100644
> > --- a/libavcodec/libx265.c
> > +++ b/libavcodec/libx265.c
> > @@ -45,6 +45,7 @@ typedef struct libx265Context {
> >  int   forced_idr;
> >  char *preset;
> >  char *tune;
> > +char *profile;
> >  char *x265_opts;
> >  } libx265Context;
> >
> > @@ -220,6 +221,13 @@ static av_cold int libx265_encode_init(AVCodecContext
> *avctx)
> >  }
> >  }
> >
> > +if (ctx->profile) {
> > +if (ctx->api->param_apply_profile(ctx->params, ctx->profile) <
> 0) {
> > +av_log(avctx, AV_LOG_ERROR, "Invalid or incompatible
> profile set : %s.\n", ctx->profile);
>
> Nit: change "set :" to "set:".
>
> Would it be possible to list the profiles with x265_profile_names
> similar to what is done for libx264?
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Lou Logan
On Tue, 5 Dec 2017 19:46:09 +0530
Gyan Doshi  wrote:

> From 13ad80871978fe7e5837863e0e2f7b7d6b356155 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi 
> Date: Tue, 5 Dec 2017 13:17:53 +0530
> Subject: [PATCH] avcodec/libx265 - Add named option to set profile
> 
> Adds call to x265_param_apply_profile after x265_param_parse.
> Added as private option since HEVC profiles other than
> Main, Main 10 and MSP in AVCodecContext are consolidated in a single
> constant.
> ---
>  libavcodec/libx265.c | 9 +
>  libavcodec/version.h | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> index 4456e300f2..4058deac1c 100644
> --- a/libavcodec/libx265.c
> +++ b/libavcodec/libx265.c
> @@ -45,6 +45,7 @@ typedef struct libx265Context {
>  int   forced_idr;
>  char *preset;
>  char *tune;
> +char *profile;
>  char *x265_opts;
>  } libx265Context;
>  
> @@ -220,6 +221,13 @@ static av_cold int libx265_encode_init(AVCodecContext 
> *avctx)
>  }
>  }
>  
> +if (ctx->profile) {
> +if (ctx->api->param_apply_profile(ctx->params, ctx->profile) < 0) {
> +av_log(avctx, AV_LOG_ERROR, "Invalid or incompatible profile set 
> : %s.\n", ctx->profile);

Nit: change "set :" to "set:".

Would it be possible to list the profiles with x265_profile_names
similar to what is done for libx264?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Gyan Doshi


On 12/5/2017 7:37 PM, Derek Buitenhuis wrote:


Patch OK with log line dropped.


Revised patch v2 attached.


Thanks,
Gyan
From 13ad80871978fe7e5837863e0e2f7b7d6b356155 Mon Sep 17 00:00:00 2001
From: Gyan Doshi 
Date: Tue, 5 Dec 2017 13:17:53 +0530
Subject: [PATCH] avcodec/libx265 - Add named option to set profile

Adds call to x265_param_apply_profile after x265_param_parse.
Added as private option since HEVC profiles other than
Main, Main 10 and MSP in AVCodecContext are consolidated in a single
constant.
---
 libavcodec/libx265.c | 9 +
 libavcodec/version.h | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 4456e300f2..4058deac1c 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -45,6 +45,7 @@ typedef struct libx265Context {
 int   forced_idr;
 char *preset;
 char *tune;
+char *profile;
 char *x265_opts;
 } libx265Context;
 
@@ -220,6 +221,13 @@ static av_cold int libx265_encode_init(AVCodecContext 
*avctx)
 }
 }
 
+if (ctx->profile) {
+if (ctx->api->param_apply_profile(ctx->params, ctx->profile) < 0) {
+av_log(avctx, AV_LOG_ERROR, "Invalid or incompatible profile set : 
%s.\n", ctx->profile);
+return AVERROR(EINVAL);
+}
+}
+
 ctx->encoder = ctx->api->encoder_open(ctx->params);
 if (!ctx->encoder) {
 av_log(avctx, AV_LOG_ERROR, "Cannot open libx265 encoder.\n");
@@ -392,6 +400,7 @@ static const AVOption options[] = {
 { "forced-idr",  "if forcing keyframes, force them as IDR frames", 
 OFFSET(forced_idr),AV_OPT_TYPE_BOOL,   { .i64 =  0 },  0,  
 1, VE },
 { "preset",  "set the x265 preset",
 OFFSET(preset),AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { "tune","set the x265 tune parameter",
 OFFSET(tune),  AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
+{ "profile", "set the x265 profile",   
 OFFSET(profile),   AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { "x265-params", "set the x265 configuration using a :-separated list of 
key=value parameters", OFFSET(x265_opts), AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { NULL }
 };
diff --git a/libavcodec/version.h b/libavcodec/version.h
index d67b689142..3b5c3000be 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@
 
 #define LIBAVCODEC_VERSION_MAJOR  58
 #define LIBAVCODEC_VERSION_MINOR   6
-#define LIBAVCODEC_VERSION_MICRO 102
+#define LIBAVCODEC_VERSION_MICRO 103
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
LIBAVCODEC_VERSION_MINOR, \
-- 
2.11.1.windows.1___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Derek Buitenhuis
On 12/5/2017 2:05 PM, Gyan Doshi wrote:
> 
> On 12/5/2017 7:21 PM, Derek Buitenhuis wrote:
> 
>> This is inconsistent with the way libx264.c does it. It calls the profile
>> function before the param parsing.
> 
>  From http://x265.readthedocs.io/en/default/cli.html#profile-level-tier
> 
> "API users must call x265_param_apply_profile() after configuring their 
> param structure. Any changes made to the param structure after this call 
> might make the encode non-compliant."
> 
> and
> 
> "Also note that x265 determines the decoder requirement profile and 
> level in three steps. First, the user configures an x265_param structure 
> with their suggested encoder options and then optionally calls 
> x265_param_apply_profile() to enforce a specific profile (main, main10, 
> etc). "

Sounds like a very good reason to me, then.

Patch OK with log line dropped.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Gyan Doshi


On 12/5/2017 7:21 PM, Derek Buitenhuis wrote:


This is inconsistent with the way libx264.c does it. It calls the profile
function before the param parsing.


From http://x265.readthedocs.io/en/default/cli.html#profile-level-tier

"API users must call x265_param_apply_profile() after configuring their 
param structure. Any changes made to the param structure after this call 
might make the encode non-compliant."


and

"Also note that x265 determines the decoder requirement profile and 
level in three steps. First, the user configures an x265_param structure 
with their suggested encoder options and then optionally calls 
x265_param_apply_profile() to enforce a specific profile (main, main10, 
etc). "


Will drop 2nd log line.

Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Derek Buitenhuis
On 12/5/2017 9:22 AM, Gyan Doshi wrote:
> Revised patch attached.

[...]

> From bbb8013e7404360139a13b58a377a29d3ca69552 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi 
> Date: Tue, 5 Dec 2017 13:17:53 +0530
> Subject: [PATCH] avcodec/libx265 - Add named option to set profile
> 
> Adds call to x265_param_apply_profile after x265_param_parse.
> Added as private option since HEVC profiles other than
> Main, Main 10 and MSP in AVCodecContext are consolidated in a single
> constant.

This is inconsistent with the way libx264.c does it. It calls the profile
function before the param parsing.

> +if (ctx->profile) {
> +if (ctx->api->param_apply_profile(ctx->params, ctx->profile) < 0) {
> +av_log(avctx, AV_LOG_ERROR, "Invalid or incompatible profile set 
> : %s.\n", ctx->profile);
> +av_log(avctx, AV_LOG_INFO, "Profile must match bit depth and 
> chroma sampling of output stream.\n");

Drop the second log line.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Bang He
i approve

On Tue, Dec 5, 2017 at 5:22 PM, Gyan Doshi  wrote:

>
> On 12/5/2017 2:33 PM, Hendrik Leppkes wrote:
>
>>
>> Please just name the option "profile" to stay consistent with all the
>> other encoders.
>>
>
> Revised patch attached.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Gyan Doshi


On 12/5/2017 2:33 PM, Hendrik Leppkes wrote:


Please just name the option "profile" to stay consistent with all the
other encoders.


Revised patch attached.
From bbb8013e7404360139a13b58a377a29d3ca69552 Mon Sep 17 00:00:00 2001
From: Gyan Doshi 
Date: Tue, 5 Dec 2017 13:17:53 +0530
Subject: [PATCH] avcodec/libx265 - Add named option to set profile

Adds call to x265_param_apply_profile after x265_param_parse.
Added as private option since HEVC profiles other than
Main, Main 10 and MSP in AVCodecContext are consolidated in a single
constant.
---
 libavcodec/libx265.c | 10 ++
 libavcodec/version.h |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 4456e300f2..64250ded62 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -45,6 +45,7 @@ typedef struct libx265Context {
 int   forced_idr;
 char *preset;
 char *tune;
+char *profile;
 char *x265_opts;
 } libx265Context;
 
@@ -220,6 +221,14 @@ static av_cold int libx265_encode_init(AVCodecContext 
*avctx)
 }
 }
 
+if (ctx->profile) {
+if (ctx->api->param_apply_profile(ctx->params, ctx->profile) < 0) {
+av_log(avctx, AV_LOG_ERROR, "Invalid or incompatible profile set : 
%s.\n", ctx->profile);
+av_log(avctx, AV_LOG_INFO, "Profile must match bit depth and 
chroma sampling of output stream.\n");
+return AVERROR(EINVAL);
+}
+}
+
 ctx->encoder = ctx->api->encoder_open(ctx->params);
 if (!ctx->encoder) {
 av_log(avctx, AV_LOG_ERROR, "Cannot open libx265 encoder.\n");
@@ -392,6 +401,7 @@ static const AVOption options[] = {
 { "forced-idr",  "if forcing keyframes, force them as IDR frames", 
 OFFSET(forced_idr),AV_OPT_TYPE_BOOL,   { .i64 =  0 },  0,  
 1, VE },
 { "preset",  "set the x265 preset",
 OFFSET(preset),AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { "tune","set the x265 tune parameter",
 OFFSET(tune),  AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
+{ "profile", "set the x265 profile",   
 OFFSET(profile),   AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { "x265-params", "set the x265 configuration using a :-separated list of 
key=value parameters", OFFSET(x265_opts), AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
 { NULL }
 };
diff --git a/libavcodec/version.h b/libavcodec/version.h
index d67b689142..3b5c3000be 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@
 
 #define LIBAVCODEC_VERSION_MAJOR  58
 #define LIBAVCODEC_VERSION_MINOR   6
-#define LIBAVCODEC_VERSION_MICRO 102
+#define LIBAVCODEC_VERSION_MICRO 103
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
LIBAVCODEC_VERSION_MINOR, \
-- 
2.11.1.windows.1___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libx265.c - Add named option to set profile

2017-12-05 Thread Hendrik Leppkes
On Tue, Dec 5, 2017 at 9:12 AM, Gyan Doshi  wrote:
> Attached patch allows users to set x265 encoding profile.
> Addresses ticket #6894.
>

Please just name the option "profile" to stay consistent with all the
other encoders.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel