[FFmpeg-devel] [PATCH v3 2/3] avcodec/libopenh264enc: make the profile configuablable correctly

2022-01-14 Thread lance . lmwang
From: Limin Wang 

due to the limitations set in d3a7bdd4ac54349aea9150a234478635d50ebd87,
you weren't able to use main profile with OpenH264 1.8, or high profile
with older versions

Reviewed-by: Martin Storsjö 
Signed-off-by: Limin Wang 
---
 libavcodec/libopenh264enc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 5b5914c..1bad233 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -220,26 +220,25 @@ static av_cold int svc_encode_init(AVCodecContext *avctx)
 #endif
 
 switch (s->profile) {
-#if OPENH264_VER_AT_LEAST(1, 8)
 case FF_PROFILE_H264_HIGH:
 param.iEntropyCodingModeFlag = 1;
 av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
 "select EProfileIdc PRO_HIGH in libopenh264.\n");
 break;
-#else
 case FF_PROFILE_H264_MAIN:
 param.iEntropyCodingModeFlag = 1;
 av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
 "select EProfileIdc PRO_MAIN in libopenh264.\n");
 break;
-#endif
-case FF_PROFILE_H264_CONSTRAINED_BASELINE:
 case FF_PROFILE_UNKNOWN:
+s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
+case FF_PROFILE_H264_CONSTRAINED_BASELINE:
 param.iEntropyCodingModeFlag = 0;
 av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
"select EProfileIdc PRO_BASELINE in libopenh264.\n");
 break;
 default:
+s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
 param.iEntropyCodingModeFlag = 0;
 av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
"select EProfileIdc PRO_BASELINE in libopenh264.\n");
@@ -251,6 +250,7 @@ static av_cold int svc_encode_init(AVCodecContext *avctx)
 param.sSpatialLayers[0].fFrameRate  = param.fMaxFrameRate;
 param.sSpatialLayers[0].iSpatialBitrate = param.iTargetBitrate;
 param.sSpatialLayers[0].iMaxSpatialBitrate  = param.iMaxBitrate;
+param.sSpatialLayers[0].uiProfileIdc= s->profile;
 
 #if OPENH264_VER_AT_LEAST(1, 7)
 if (avctx->sample_aspect_ratio.num && avctx->sample_aspect_ratio.den) {
-- 
1.8.3.1

___
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 v3 2/3] avcodec/libopenh264enc: make the profile configuablable correctly

2022-01-14 Thread Martin Storsjö

On Fri, 14 Jan 2022, lance.lmw...@gmail.com wrote:


From: Limin Wang 

due to the limitations set in d3a7bdd4ac54349aea9150a234478635d50ebd87,
you weren't able to use main profile with OpenH264 1.8, or high profile
with older versions

Reviewed-by: Martin Storsjö 
Signed-off-by: Limin Wang 
---
libavcodec/libopenh264enc.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 5b5914c..1bad233 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -220,26 +220,25 @@ static av_cold int svc_encode_init(AVCodecContext *avctx)
#endif

switch (s->profile) {
-#if OPENH264_VER_AT_LEAST(1, 8)
case FF_PROFILE_H264_HIGH:
param.iEntropyCodingModeFlag = 1;
av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
"select EProfileIdc PRO_HIGH in libopenh264.\n");
break;
-#else
case FF_PROFILE_H264_MAIN:
param.iEntropyCodingModeFlag = 1;
av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
"select EProfileIdc PRO_MAIN in libopenh264.\n");
break;
-#endif
-case FF_PROFILE_H264_CONSTRAINED_BASELINE:
case FF_PROFILE_UNKNOWN:
+s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
+case FF_PROFILE_H264_CONSTRAINED_BASELINE:


I don't think it's specifically necessary to move the case labels around 
like this; you could just leave both case labels above it, and the 
assignment would be a no-op for one case. (No need to resend the patch for 
that, do whichever form you prefer.)



param.iEntropyCodingModeFlag = 0;
av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
break;
default:
+s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
param.iEntropyCodingModeFlag = 0;
av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
@@ -251,6 +250,7 @@ static av_cold int svc_encode_init(AVCodecContext *avctx)
param.sSpatialLayers[0].fFrameRate  = param.fMaxFrameRate;
param.sSpatialLayers[0].iSpatialBitrate = param.iTargetBitrate;
param.sSpatialLayers[0].iMaxSpatialBitrate  = param.iMaxBitrate;
+param.sSpatialLayers[0].uiProfileIdc= s->profile;


Thanks, this seems sensible to me. This is ok if you've tested that 
setting profile to FF_PROFILE_H264_CONSTRAINED_BASELINE (as opposed to 
PRO_BASELINE) does work with both some old and some recent version of 
OpenH264.


// 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".


Re: [FFmpeg-devel] [PATCH v3 2/3] avcodec/libopenh264enc: make the profile configuablable correctly

2022-01-14 Thread lance . lmwang
On Fri, Jan 14, 2022 at 02:07:26PM +0200, Martin Storsjö wrote:
> On Fri, 14 Jan 2022, lance.lmw...@gmail.com wrote:
> 
> > From: Limin Wang 
> > 
> > due to the limitations set in d3a7bdd4ac54349aea9150a234478635d50ebd87,
> > you weren't able to use main profile with OpenH264 1.8, or high profile
> > with older versions
> > 
> > Reviewed-by: Martin Storsjö 
> > Signed-off-by: Limin Wang 
> > ---
> > libavcodec/libopenh264enc.c | 8 
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index 5b5914c..1bad233 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -220,26 +220,25 @@ static av_cold int svc_encode_init(AVCodecContext 
> > *avctx)
> > #endif
> > 
> > switch (s->profile) {
> > -#if OPENH264_VER_AT_LEAST(1, 8)
> > case FF_PROFILE_H264_HIGH:
> > param.iEntropyCodingModeFlag = 1;
> > av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
> > "select EProfileIdc PRO_HIGH in libopenh264.\n");
> > break;
> > -#else
> > case FF_PROFILE_H264_MAIN:
> > param.iEntropyCodingModeFlag = 1;
> > av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
> > "select EProfileIdc PRO_MAIN in libopenh264.\n");
> > break;
> > -#endif
> > -case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> > case FF_PROFILE_UNKNOWN:
> > +s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
> > +case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> 
> I don't think it's specifically necessary to move the case labels around
> like this; you could just leave both case labels above it, and the
> assignment would be a no-op for one case. (No need to resend the patch for
> that, do whichever form you prefer.)

OK, will not move the case lables as suggestion.

> 
> > param.iEntropyCodingModeFlag = 0;
> > av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
> >"select EProfileIdc PRO_BASELINE in libopenh264.\n");
> > break;
> > default:
> > +s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
> > param.iEntropyCodingModeFlag = 0;
> > av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
> >"select EProfileIdc PRO_BASELINE in libopenh264.\n");
> > @@ -251,6 +250,7 @@ static av_cold int svc_encode_init(AVCodecContext 
> > *avctx)
> > param.sSpatialLayers[0].fFrameRate  = param.fMaxFrameRate;
> > param.sSpatialLayers[0].iSpatialBitrate = param.iTargetBitrate;
> > param.sSpatialLayers[0].iMaxSpatialBitrate  = param.iMaxBitrate;
> > +param.sSpatialLayers[0].uiProfileIdc= s->profile;
> 
> Thanks, this seems sensible to me. This is ok if you've tested that setting
> profile to FF_PROFILE_H264_CONSTRAINED_BASELINE (as opposed to PRO_BASELINE)
> does work with both some old and some recent version of OpenH264.

Yes, have tested with v1.4.0, v1.7.0 and master.

> 
> // Martin


-- 
Thanks,
Limin Wang
___
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".