diff mbox series

[FFmpeg-devel,v4,7/9] lavc/libopenh264enc: add profile high option support

Message ID 1586926427-20170-8-git-send-email-linjie.fu@intel.com
State New
Headers show
Series Enhancement for libopenh264 encoder
Related show

Checks

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

Commit Message

Fu, Linjie April 15, 2020, 4:53 a.m. UTC
Add support for PRO_HIGH/PRO_BASELINE in SVC Encoding extention mode,
which determined by iEntropyCodingModeFlag in ParamTranscode().

<https://github.com/cisco/openh264/blob/master/codec/encoder/core/inc/param_svc.h#L394>

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavcodec/libopenh264enc.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

Comments

Martin Storsjö April 27, 2020, 8:08 p.m. UTC | #1
On Wed, 15 Apr 2020, Linjie Fu wrote:

> Add support for PRO_HIGH/PRO_BASELINE in SVC Encoding extention mode,
> which determined by iEntropyCodingModeFlag in ParamTranscode().
>
> <https://github.com/cisco/openh264/blob/master/codec/encoder/core/inc/param_svc.h#L394>
>
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
> libavcodec/libopenh264enc.c | 32 ++++++++++++++++++++++++++------
> 1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index a7c389e..0fe8cf4 100644
> --- a/libavcodec/libopenh264enc.c
> +++ b/libavcodec/libopenh264enc.c
> @@ -42,7 +42,7 @@ typedef struct SVCContext {
>     ISVCEncoder *encoder;
>     int slice_mode;
>     int loopfilter;
> -    char *profile;
> +    int profile;
>     int max_nal_size;
>     int skip_frames;
>     int skipped;
> @@ -73,7 +73,11 @@ static const AVOption options[] = {
> #endif
> #endif
>     { "loopfilter", "enable loop filter", OFFSET(loopfilter), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, VE },
> -    { "profile", "set profile restrictions", OFFSET(profile), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE },
> +    { "profile", "Set profile restrictions", OFFSET(profile), AV_OPT_TYPE_INT, { .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 0xffff, VE, "profile" },
> +#define PROFILE(name, value)  name, NULL, 0, AV_OPT_TYPE_CONST, { .i64 = value }, 0, 0, VE, "profile"
> +        { PROFILE("constrained_baseline", FF_PROFILE_H264_CONSTRAINED_BASELINE) },
> +        { PROFILE("high",                 FF_PROFILE_H264_HIGH) },
> +#undef PROFILE
>     { "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
>     { "allow_skip_frames", "allow skipping frames to hit the target bitrate", OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
>     { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE },
> @@ -109,12 +113,28 @@ static av_cold int svc_encode_init_profile(AVCodecContext *avctx, SEncParamExt *
> {
>     SVCContext *s = avctx->priv_data;
> 
> -    if (s->profile && !strcmp(s->profile, "main"))
> -        param->iEntropyCodingModeFlag = 1; //< 0:CAVLC  1:CABAC
> -    else if (!s->profile && s->cabac)
> +    if (s->profile == FF_PROFILE_UNKNOWN)
> +        s->profile = s->cabac ? FF_PROFILE_H264_HIGH :
> +                                FF_PROFILE_H264_CONSTRAINED_BASELINE;
> +
> +    switch (s->profile) {
> +    case FF_PROFILE_H264_HIGH:
>         param->iEntropyCodingModeFlag = 1;
> -    else
> +        av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
> +               "select EProfileIdc PRO_HIGH in libopenh264.\n");
> +        break;
> +    case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> +    case FF_PROFILE_UNKNOWN:
> +        param->iEntropyCodingModeFlag = 0;
> +        av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
> +               "select EProfileIdc PRO_BASELINE in libopenh264.\n");
> +        break;
> +    default:
>         param->iEntropyCodingModeFlag = 0;
> +        av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
> +               "select EProfileIdc PRO_BASELINE in libopenh264.\n");
> +        break;
> +    }
>
>     return 0;
> }
> -- 
> 2.7.4

This is fine I guess - apparently it changed in OpenH264 1.8, before that, 
setting iEntropyCodingModeFlag = 1 got main profile, not high.

The commit message feels a bit misleading for what the commit really does 
though. The commit message says "add ... support", which sounds like it 
would be a pure addition, while it is a bit of a rewrite, and actually 
removes support for "-profile main" (even though it might have given 
profile high with current OpenH264 versions).

Maybe this would be clearer:
----
libopenh264enc: Rewrite profile handling

Support the profiles "constrained_baseline" and "high". Previously, the 
encoder wrapper supported the parameter value "main", but with recent 
versions of OpenH264, the produed bitstream signaled the profile high 
anyway.
---

That's at least clear with what the change actually does.

// Martin
Fu, Linjie May 2, 2020, 3:36 a.m. UTC | #2
> From: Martin Storsjö <martin@martin.st>
> Sent: Tuesday, April 28, 2020 04:09
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Cc: Fu, Linjie <linjie.fu@intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH v4 7/9] lavc/libopenh264enc: add profile
> high option support
> 
> On Wed, 15 Apr 2020, Linjie Fu wrote:
> 
> > Add support for PRO_HIGH/PRO_BASELINE in SVC Encoding extention
> mode,
> > which determined by iEntropyCodingModeFlag in ParamTranscode().
> >
> >
> <https://github.com/cisco/openh264/blob/master/codec/encoder/core/inc/
> param_svc.h#L394>
> >
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > ---
> > libavcodec/libopenh264enc.c | 32 ++++++++++++++++++++++++++------
> > 1 file changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index a7c389e..0fe8cf4 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -42,7 +42,7 @@ typedef struct SVCContext {
> >     ISVCEncoder *encoder;
> >     int slice_mode;
> >     int loopfilter;
> > -    char *profile;
> > +    int profile;
> >     int max_nal_size;
> >     int skip_frames;
> >     int skipped;
> > @@ -73,7 +73,11 @@ static const AVOption options[] = {
> > #endif
> > #endif
> >     { "loopfilter", "enable loop filter", OFFSET(loopfilter), AV_OPT_TYPE_INT,
> { .i64 = 1 }, 0, 1, VE },
> > -    { "profile", "set profile restrictions", OFFSET(profile),
> AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE },
> > +    { "profile", "Set profile restrictions", OFFSET(profile), AV_OPT_TYPE_INT,
> { .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 0xffff, VE,
> "profile" },
> > +#define PROFILE(name, value)  name, NULL, 0, AV_OPT_TYPE_CONST,
> { .i64 = value }, 0, 0, VE, "profile"
> > +        { PROFILE("constrained_baseline",
> FF_PROFILE_H264_CONSTRAINED_BASELINE) },
> > +        { PROFILE("high",                 FF_PROFILE_H264_HIGH) },
> > +#undef PROFILE
> >     { "max_nal_size", "set maximum NAL size in bytes",
> OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
> >     { "allow_skip_frames", "allow skipping frames to hit the target bitrate",
> OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> >     { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 1, VE },
> > @@ -109,12 +113,28 @@ static av_cold int
> svc_encode_init_profile(AVCodecContext *avctx, SEncParamExt *
> > {
> >     SVCContext *s = avctx->priv_data;
> >
> > -    if (s->profile && !strcmp(s->profile, "main"))
> > -        param->iEntropyCodingModeFlag = 1; //< 0:CAVLC  1:CABAC
> > -    else if (!s->profile && s->cabac)
> > +    if (s->profile == FF_PROFILE_UNKNOWN)
> > +        s->profile = s->cabac ? FF_PROFILE_H264_HIGH :
> > +                                FF_PROFILE_H264_CONSTRAINED_BASELINE;
> > +
> > +    switch (s->profile) {
> > +    case FF_PROFILE_H264_HIGH:
> >         param->iEntropyCodingModeFlag = 1;
> > -    else
> > +        av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
> > +               "select EProfileIdc PRO_HIGH in libopenh264.\n");
> > +        break;
> > +    case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> > +    case FF_PROFILE_UNKNOWN:
> > +        param->iEntropyCodingModeFlag = 0;
> > +        av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
> > +               "select EProfileIdc PRO_BASELINE in libopenh264.\n");
> > +        break;
> > +    default:
> >         param->iEntropyCodingModeFlag = 0;
> > +        av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
> > +               "select EProfileIdc PRO_BASELINE in libopenh264.\n");
> > +        break;
> > +    }
> >
> >     return 0;
> > }
> > --
> > 2.7.4
> 
> This is fine I guess - apparently it changed in OpenH264 1.8, before that,
> setting iEntropyCodingModeFlag = 1 got main profile, not high.

Yes, main profile was somehow removed in [1] v1.7, while high profile
was introduced in [2] v1.8. 

> The commit message feels a bit misleading for what the commit really does
> though. The commit message says "add ... support", which sounds like it
> would be a pure addition, while it is a bit of a rewrite, and actually
> removes support for "-profile main" (even though it might have given
> profile high with current OpenH264 versions).

> Maybe this would be clearer:
> ----
> libopenh264enc: Rewrite profile handling
> 
> Support the profiles "constrained_baseline" and "high". Previously, the
> encoder wrapper supported the parameter value "main", but with recent
> versions of OpenH264, the produced bitstream signaled the profile high
> anyway.
> ---
> 
> That's at least clear with what the change actually does.
Yes, this would be more clear and reasonable, will update like this.

Actually, main profile currently seems to be available in ParamBaseTranscode() [3]
For SEncParamBase (did't try yet). IMHO it'll be better to add the PRO_MAIN support
back later.

[1] https://github.com/cisco/openh264/commit/f09c85f45d52c91234482b97b34fe01dadd002c7
[2] https://github.com/cisco/openh264/commit/9fc2bbf1fe73fdbb1ea804705efe7ac40bb0d773
[3] https://github.com/cisco/openh264/blob/master/codec/encoder/core/inc/param_svc.h#L249

- Linjie
diff mbox series

Patch

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index a7c389e..0fe8cf4 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -42,7 +42,7 @@  typedef struct SVCContext {
     ISVCEncoder *encoder;
     int slice_mode;
     int loopfilter;
-    char *profile;
+    int profile;
     int max_nal_size;
     int skip_frames;
     int skipped;
@@ -73,7 +73,11 @@  static const AVOption options[] = {
 #endif
 #endif
     { "loopfilter", "enable loop filter", OFFSET(loopfilter), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, VE },
-    { "profile", "set profile restrictions", OFFSET(profile), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE },
+    { "profile", "Set profile restrictions", OFFSET(profile), AV_OPT_TYPE_INT, { .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 0xffff, VE, "profile" },
+#define PROFILE(name, value)  name, NULL, 0, AV_OPT_TYPE_CONST, { .i64 = value }, 0, 0, VE, "profile"
+        { PROFILE("constrained_baseline", FF_PROFILE_H264_CONSTRAINED_BASELINE) },
+        { PROFILE("high",                 FF_PROFILE_H264_HIGH) },
+#undef PROFILE
     { "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
     { "allow_skip_frames", "allow skipping frames to hit the target bitrate", OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
     { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE },
@@ -109,12 +113,28 @@  static av_cold int svc_encode_init_profile(AVCodecContext *avctx, SEncParamExt *
 {
     SVCContext *s = avctx->priv_data;
 
-    if (s->profile && !strcmp(s->profile, "main"))
-        param->iEntropyCodingModeFlag = 1; //< 0:CAVLC  1:CABAC
-    else if (!s->profile && s->cabac)
+    if (s->profile == FF_PROFILE_UNKNOWN)
+        s->profile = s->cabac ? FF_PROFILE_H264_HIGH :
+                                FF_PROFILE_H264_CONSTRAINED_BASELINE;
+
+    switch (s->profile) {
+    case FF_PROFILE_H264_HIGH:
         param->iEntropyCodingModeFlag = 1;
-    else
+        av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
+               "select EProfileIdc PRO_HIGH in libopenh264.\n");
+        break;
+    case FF_PROFILE_H264_CONSTRAINED_BASELINE:
+    case FF_PROFILE_UNKNOWN:
+        param->iEntropyCodingModeFlag = 0;
+        av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
+               "select EProfileIdc PRO_BASELINE in libopenh264.\n");
+        break;
+    default:
         param->iEntropyCodingModeFlag = 0;
+        av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
+               "select EProfileIdc PRO_BASELINE in libopenh264.\n");
+        break;
+    }
 
     return 0;
 }