[FFmpeg-devel] [PATCH v4 8/9] lavc/libopenh264enc: allow specifying the profile through AVCodecContext

[Martin Storsjö]

On Wed, 15 Apr 2020, Linjie Fu wrote:

> Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> ---
> libavcodec/libopenh264enc.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index 0fe8cf4..4d337d2 100644
> --- a/libavcodec/libopenh264enc.c
> +++ b/libavcodec/libopenh264enc.c
> @@ -113,6 +113,22 @@ static av_cold int svc_encode_init_profile(AVCodecContext *avctx, SEncParamExt *
> {
>     SVCContext *s = avctx->priv_data;
> 
> +    /* Allow specifying the libopenh264 profile through AVCodecContext. */
> +    if (FF_PROFILE_UNKNOWN == s->profile &&
> +        FF_PROFILE_UNKNOWN != avctx->profile)
> +        switch (avctx->profile) {
> +        case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> +            s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
> +            break;
> +        case FF_PROFILE_H264_HIGH:
> +            s->profile = FF_PROFILE_H264_HIGH;
> +            break;
> +        default:
> +            av_log(avctx, AV_LOG_WARNING,
> +                   "Unsupported avctx->profile: %d.\n", avctx->profile);
> +            break;
> +        }
> +
>     if (s->profile == FF_PROFILE_UNKNOWN)
>         s->profile = s->cabac ? FF_PROFILE_H264_HIGH :
>                                 FF_PROFILE_H264_CONSTRAINED_BASELINE;

With this in place, why even do the previous commit, why not just go for 
only using avctx->profile? Although as far as I can see, that field 
doesn't seem to have string options for sepcifying H264 profiles, so it 
can only be set via code or by specifying a numberic value - is that 
right?

Wouldn't it just be best to add the H264 profiles to options_table.h to 
keep allowing it to be set via a string, but remove the internal 
s->profile field here, as patch 7/9 already breaks handling of the profile 
field by stopping recognizing "main" and only recognizing "high" anyway.

// Martin