| Message ID | 20200506053753.2194-1-linjie.fu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,RFC] lavc/options_table: Add basic candidate h264 profiles | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Wed, 6 May 2020, Linjie Fu wrote: > Allows specifying avctx->profile with string type profile for > h264 encoders. > > Private field/option "profile" may be abled to be removed for basic > h264 profiles, directly for encoders like libopenh264/h264_vaapi, > or with an map to hardware profile like h264_qsv. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > One concern is some encoders have options for specific profiles, > like "high444p" for nvenc_h264 and "constrained_high" for h264_amf, > hence they may not be able to remove the private option easily. > > Please help to comment. > > libavcodec/options_table.h | 4 ++++ > 1 file changed, 4 insertions(+) This change in itself looks sensible to me (but I'd like for someone else to comment as well). Even if it might not be able to get rid of the private option for all encoders, it should at least simplify the easiest cases. // Martin
On Wed, May 6, 2020 at 5:40 PM Martin Storsjö <martin@martin.st> wrote: > > On Wed, 6 May 2020, Linjie Fu wrote: > > > Allows specifying avctx->profile with string type profile for > > h264 encoders. > > > > Private field/option "profile" may be abled to be removed for basic > > h264 profiles, directly for encoders like libopenh264/h264_vaapi, > > or with an map to hardware profile like h264_qsv. > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > One concern is some encoders have options for specific profiles, > > like "high444p" for nvenc_h264 and "constrained_high" for h264_amf, > > hence they may not be able to remove the private option easily. > > > > Please help to comment. > > > > libavcodec/options_table.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > This change in itself looks sensible to me (but I'd like for someone else > to comment as well). Even if it might not be able to get rid of the > private option for all encoders, it should at least simplify the easiest > cases. > > // Martin I think we have a discussion about this case, https://patchwork.ffmpeg.org/project/ffmpeg/patch/fa14da65-9e1a-c74b-b3cb-46fc5f3ea932@gmail.com/
On Wed, May 6, 2020 at 6:04 PM mypopy@gmail.com <mypopy@gmail.com> wrote: > > On Wed, May 6, 2020 at 5:40 PM Martin Storsjö <martin@martin.st> wrote: > > > > On Wed, 6 May 2020, Linjie Fu wrote: > > > > > Allows specifying avctx->profile with string type profile for > > > h264 encoders. > > > > > > Private field/option "profile" may be abled to be removed for basic > > > h264 profiles, directly for encoders like libopenh264/h264_vaapi, > > > or with an map to hardware profile like h264_qsv. > > > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > > --- > > > One concern is some encoders have options for specific profiles, > > > like "high444p" for nvenc_h264 and "constrained_high" for h264_amf, > > > hence they may not be able to remove the private option easily. > > > > > > Please help to comment. > > > > > > libavcodec/options_table.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > This change in itself looks sensible to me (but I'd like for someone else > > to comment as well). Even if it might not be able to get rid of the > > private option for all encoders, it should at least simplify the easiest > > cases. > > > > // Martin > I think we have a discussion about this case, > https://patchwork.ffmpeg.org/project/ffmpeg/patch/fa14da65-9e1a-c74b-b3cb-46fc5f3ea932@gmail.com/ I agree with Hendrik Leppkes, and I think "main" more natural than "h264_main" for 264 codec profile from a user viewpoint, both of command line tools or application program interface
> From: mypopy@gmail.com <mypopy@gmail.com> > Sent: Wednesday, May 6, 2020 18:13 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Cc: Fu, Linjie <linjie.fu@intel.com> > Subject: Re: [FFmpeg-devel] [PATCH, RFC] lavc/options_table: Add basic > candidate h264 profiles > > On Wed, May 6, 2020 at 6:04 PM mypopy@gmail.com <mypopy@gmail.com> > wrote: > > > > On Wed, May 6, 2020 at 5:40 PM Martin Storsjö <martin@martin.st> wrote: > > > > > > On Wed, 6 May 2020, Linjie Fu wrote: > > > > > > > Allows specifying avctx->profile with string type profile for > > > > h264 encoders. > > > > > > > > Private field/option "profile" may be abled to be removed for basic > > > > h264 profiles, directly for encoders like libopenh264/h264_vaapi, > > > > or with an map to hardware profile like h264_qsv. > > > > > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > > > --- > > > > One concern is some encoders have options for specific profiles, > > > > like "high444p" for nvenc_h264 and "constrained_high" for h264_amf, > > > > hence they may not be able to remove the private option easily. > > > > > > > > Please help to comment. > > > > > > > > libavcodec/options_table.h | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > This change in itself looks sensible to me (but I'd like for someone else > > > to comment as well). Even if it might not be able to get rid of the > > > private option for all encoders, it should at least simplify the easiest > > > cases. > > > > > > // Martin > > I think we have a discussion about this case, > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/fa14da65-9e1a- > c74b-b3cb-46fc5f3ea932@gmail.com/ Ahh, thanks for this input since it seems to be well-discussed. > I agree with Hendrik Leppkes, and I think "main" more natural than > "h264_main" for 264 codec profile from a user viewpoint, both of > command line tools or application program interface Indeed, being natural makes sense. (otherwise we would have used numeric values instead) - Linjie
On Wed, May 06, 2020 at 06:13:10PM +0800, mypopy@gmail.com wrote: > On Wed, May 6, 2020 at 6:04 PM mypopy@gmail.com <mypopy@gmail.com> wrote: > > > > On Wed, May 6, 2020 at 5:40 PM Martin Storsjö <martin@martin.st> wrote: > > > > > > On Wed, 6 May 2020, Linjie Fu wrote: > > > > > > > Allows specifying avctx->profile with string type profile for > > > > h264 encoders. > > > > > > > > Private field/option "profile" may be abled to be removed for basic > > > > h264 profiles, directly for encoders like libopenh264/h264_vaapi, > > > > or with an map to hardware profile like h264_qsv. > > > > > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > > > --- > > > > One concern is some encoders have options for specific profiles, > > > > like "high444p" for nvenc_h264 and "constrained_high" for h264_amf, > > > > hence they may not be able to remove the private option easily. > > > > > > > > Please help to comment. > > > > > > > > libavcodec/options_table.h | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > This change in itself looks sensible to me (but I'd like for someone else > > > to comment as well). Even if it might not be able to get rid of the > > > private option for all encoders, it should at least simplify the easiest > > > cases. > > > > > > // Martin > > I think we have a discussion about this case, > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/fa14da65-9e1a-c74b-b3cb-46fc5f3ea932@gmail.com/ > > I agree with Hendrik Leppkes, and I think "main" more natural than > "h264_main" for 264 codec profile from a user viewpoint, both of > command line tools or application program interface So what's the conclusion? I have submit a patch to add mpeg2 profile 52 and Marton prefer to use global profile with mpeg2_xxxx. By Hendrik's comments, I think it's more reasonable to use private profile. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > lance.lmwang@gmail.com > Sent: Wednesday, May 6, 2020 22:57 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH, RFC] lavc/options_table: Add basic > candidate h264 profiles > > On Wed, May 06, 2020 at 06:13:10PM +0800, mypopy@gmail.com wrote: > > On Wed, May 6, 2020 at 6:04 PM mypopy@gmail.com > <mypopy@gmail.com> wrote: > > > > > > On Wed, May 6, 2020 at 5:40 PM Martin Storsjö <martin@martin.st> > wrote: > > > > > > > > On Wed, 6 May 2020, Linjie Fu wrote: > > > > > > > > > Allows specifying avctx->profile with string type profile for > > > > > h264 encoders. > > > > > > > > > > Private field/option "profile" may be abled to be removed for basic > > > > > h264 profiles, directly for encoders like libopenh264/h264_vaapi, > > > > > or with an map to hardware profile like h264_qsv. > > > > > > > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > > > > --- > > > > > One concern is some encoders have options for specific profiles, > > > > > like "high444p" for nvenc_h264 and "constrained_high" for h264_amf, > > > > > hence they may not be able to remove the private option easily. > > > > > > > > > > Please help to comment. > > > > > > > > > > libavcodec/options_table.h | 4 ++++ > > > > > 1 file changed, 4 insertions(+) > > > > > > > > This change in itself looks sensible to me (but I'd like for someone else > > > > to comment as well). Even if it might not be able to get rid of the > > > > private option for all encoders, it should at least simplify the easiest > > > > cases. > > > > > > > > // Martin > > > I think we have a discussion about this case, > > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/fa14da65-9e1a- > c74b-b3cb-46fc5f3ea932@gmail.com/ > > > > I agree with Hendrik Leppkes, and I think "main" more natural than > > "h264_main" for 264 codec profile from a user viewpoint, both of > > command line tools or application program interface > > So what's the conclusion? I have submit a patch to add mpeg2 profile > 52 and Marton prefer to use global profile with mpeg2_xxxx. By Hendrik's > comments, I think it's more reasonable to use private profile. I'd also prefer an option without prefix which seems more natural, especially for the codecs that already have implemented it as a private option. Otherwise it seems to be a sudden break for users to use "h264_main"/"mepg2_main" instead of "main" unless we declared some deprecated flags and get rid of the private options later. Please help to comment. - Linjie
On Wed, 6 May 2020, Fu, Linjie wrote: >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of >> lance.lmwang@gmail.com >> Sent: Wednesday, May 6, 2020 22:57 >> To: ffmpeg-devel@ffmpeg.org >> Subject: Re: [FFmpeg-devel] [PATCH, RFC] lavc/options_table: Add basic >> candidate h264 profiles >> >> On Wed, May 06, 2020 at 06:13:10PM +0800, mypopy@gmail.com wrote: >>> On Wed, May 6, 2020 at 6:04 PM mypopy@gmail.com >> <mypopy@gmail.com> wrote: >>>> >>>> On Wed, May 6, 2020 at 5:40 PM Martin Storsjö <martin@martin.st> >> wrote: >>>>> >>>>> On Wed, 6 May 2020, Linjie Fu wrote: >>>>> >>>>>> Allows specifying avctx->profile with string type profile for >>>>>> h264 encoders. >>>>>> >>>>>> Private field/option "profile" may be abled to be removed for basic >>>>>> h264 profiles, directly for encoders like libopenh264/h264_vaapi, >>>>>> or with an map to hardware profile like h264_qsv. >>>>>> >>>>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com> >>>>>> --- >>>>>> One concern is some encoders have options for specific profiles, >>>>>> like "high444p" for nvenc_h264 and "constrained_high" for h264_amf, >>>>>> hence they may not be able to remove the private option easily. >>>>>> >>>>>> Please help to comment. >>>>>> >>>>>> libavcodec/options_table.h | 4 ++++ >>>>>> 1 file changed, 4 insertions(+) >>>>> >>>>> This change in itself looks sensible to me (but I'd like for someone else >>>>> to comment as well). Even if it might not be able to get rid of the >>>>> private option for all encoders, it should at least simplify the easiest >>>>> cases. >>>>> >>>>> // Martin >>>> I think we have a discussion about this case, >>>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/fa14da65-9e1a- >> c74b-b3cb-46fc5f3ea932@gmail.com/ >>> >>> I agree with Hendrik Leppkes, and I think "main" more natural than >>> "h264_main" for 264 codec profile from a user viewpoint, both of >>> command line tools or application program interface >> >> So what's the conclusion? I have submit a patch to add mpeg2 profile >> 52 and Marton prefer to use global profile with mpeg2_xxxx. By Hendrik's >> comments, I think it's more reasonable to use private profile. > > I'd also prefer an option without prefix which seems more natural, especially > for the codecs that already have implemented it as a private option. Otherwise > it seems to be a sudden break for users to use "h264_main"/"mepg2_main" > instead of "main" unless we declared some deprecated flags and get rid of > the private options later. > I was against the private option because there is no clear path in what to support in the future. Storing the profile in two places is problematic becuase priority between the two is not consistently enforced. So I guess for some codecs the private profile will have priority, for others, the avctx profile. If libavcodec should store all profiles for a codec, can't we make the profile option a string and do the parsing later from code? This way we can eliminate the per-codec profile options and use the shorter options at the same time them having different meaning according to the used codec. I can wrap up a patch if this seems like a viable way forward. Thanks, Marton
diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h index 54366747ca..8d41f2755c 100644 --- a/libavcodec/options_table.h +++ b/libavcodec/options_table.h @@ -282,6 +282,10 @@ static const AVOption avcodec_options[] = { {"mpeg4_asp", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_PROFILE_MPEG4_ADVANCED_SIMPLE }, INT_MIN, INT_MAX, V|E, "profile"}, {"main10", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_PROFILE_HEVC_MAIN_10 }, INT_MIN, INT_MAX, V|E, "profile"}, {"msbc", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_PROFILE_SBC_MSBC }, INT_MIN, INT_MAX, A|E, "profile"}, +{"h264_constrained_baseline", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_PROFILE_H264_CONSTRAINED_BASELINE }, INT_MIN, INT_MAX, V|E, "profile"}, +{"h264_baseline", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_PROFILE_H264_BASELINE }, INT_MIN, INT_MAX, V|E, "profile"}, +{"h264_main", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_PROFILE_H264_MAIN }, INT_MIN, INT_MAX, V|E, "profile"}, +{"h264_high", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_PROFILE_H264_HIGH }, INT_MIN, INT_MAX, V|E, "profile"}, {"level", NULL, OFFSET(level), AV_OPT_TYPE_INT, {.i64 = FF_LEVEL_UNKNOWN }, INT_MIN, INT_MAX, V|A|E, "level"}, {"unknown", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_LEVEL_UNKNOWN }, INT_MIN, INT_MAX, V|A|E, "level"}, {"lowres", "decode at 1= 1/2, 2=1/4, 3=1/8 resolutions", OFFSET(lowres), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, INT_MAX, V|A|D},
Allows specifying avctx->profile with string type profile for h264 encoders. Private field/option "profile" may be abled to be removed for basic h264 profiles, directly for encoders like libopenh264/h264_vaapi, or with an map to hardware profile like h264_qsv. Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- One concern is some encoders have options for specific profiles, like "high444p" for nvenc_h264 and "constrained_high" for h264_amf, hence they may not be able to remove the private option easily. Please help to comment. libavcodec/options_table.h | 4 ++++ 1 file changed, 4 insertions(+)