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

2020-05-01 Thread Fu, Linjie
> 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

2020-04-27 Thread Martin Storsjö

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

2020-04-14 Thread Linjie Fu
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".