| Message ID | 1586926427-20170-8-git-send-email-linjie.fu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enhancement for libopenh264 encoder | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
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
> 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 --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; }
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(-)