Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/libopenh264enc: allow specifying the profile through AVCodecContext
> From: ffmpeg-devel On Behalf Of > Martin Storsjö > Sent: Tuesday, April 28, 2020 04:13 > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Cc: Fu, Linjie > Subject: Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/libopenh264enc: allow > specifying the profile through AVCodecContext > > On Wed, 15 Apr 2020, Linjie Fu wrote: > > > Signed-off-by: Linjie Fu > > --- > > 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? Yes, code/numberic value works in this way, however maybe not straight forward enough for user. > 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. Most sw codecs like libx264/libx265 and hw codecs like h264_vaapi/h264_nvenc have the logic of : 1. derive the settings from avctx->profile which is determined in options_table.h; 2. If not, check the private option for specific encoder; And specifically for libopenh264enc, we allow one more logic: 3. determine the profile by s->cabac; I admit it'll be good if settled this down to parse all possible profiles for each codec (maybe according to libavcodec/profiles.c), hence we may remove similar/redundant logics in specific encoder(x264/h264_vaapi/h264_nvenc). Please help to comment whether it makes sense for you all, since this would impact many codecs. - Linjie ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/libopenh264enc: allow specifying the profile through AVCodecContext
On Wed, 15 Apr 2020, Linjie Fu wrote: Signed-off-by: Linjie Fu --- 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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v4 8/9] lavc/libopenh264enc: allow specifying the profile through AVCodecContext
Signed-off-by: Linjie Fu --- 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; -- 2.7.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".