Re: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit rate control select support
> From: Martin Storsjö
> Sent: Wednesday, April 29, 2020 03:18
> To: Fu, Linjie
> Cc: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: RE: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit
> rate control select support
>
> On Tue, 28 Apr 2020, Fu, Linjie wrote:
>
> >> From: Martin Storsjö
> >> Sent: Tuesday, April 28, 2020 18:31
> >> To: FFmpeg development discussions and patches >> de...@ffmpeg.org>
> >> Cc: Fu, Linjie
> >> Subject: Re: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit
> >> rate control select support
> >>
> >> On Tue, 28 Apr 2020, Linjie Fu wrote:
> >>
> >> We don't need checks for 1.2 - the initial version of openh264 that we
> >> support is 1.3, so we only need checks for 1.4 and newer.
> >
> > Ahh, didn't noticed this until checked the commit message of
> > 8a3d9ca603f4d15ecaa9ca379cbaab4ecaec8ce4.
> >
> > IMHO it seems to be better if we add this version check in the configure as
> well.
> > (Did a quick verification with version modified in pkgconfig to 1.1.0,
> configure is
> > still good for libopenh264 )
>
> We don't need an explicit version check for 1.3 in configure - we require
> that WelsGetCodecVersion can be found in the configure check, and that
> function was added in 1.3 (explicitly for the purpose so that we can check
> that the version that we have linked is the same as we compiled against).
>
Checked and confirmed that you are right, WelsGetCodecVersion is enough
for 1.3.0 version check, thanks for elaborations.
> >
> >>> +#if OPENH264_VER_AT_LEAST(1, 4)
> >>> +{ "timestamp", "bit rate control based on timestamp",
> >> 0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE,
> >> "rc_mode" },
> >>> +#endif
> >>> +
> >>> { NULL }
> >>> };
> >>
> >> This commit seems to lack something that actually sets the rc_mode value
> >> in the parameters struct though - wasn't that part of the previous version
> >> of the patch?
> >>
> > Did you mean setting rc_mode through parameters like bit_rate/qp?
> > This patch enables explicitly setting the rc_mode as part of that logic.
>
> No, I didn't mean that.
>
> The previous version of this patch had the following change:
>
> -param.iRCMode= RC_QUALITY_MODE;
> +param.iRCMode= s->rc_mode;
>
> Now I don't find that part any longer in this patch - and without that
> change, the rest of the commit is pretty pointless, no?
it's somehow missed while rebasing the set, thanks.
- 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 v5 3/5] lavc/libopenh264enc: add bit rate control select support
On Tue, 28 Apr 2020, Fu, Linjie wrote:
From: Martin Storsjö
Sent: Tuesday, April 28, 2020 18:31
To: FFmpeg development discussions and patches
Cc: Fu, Linjie
Subject: Re: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit
rate control select support
On Tue, 28 Apr 2020, Linjie Fu wrote:
We don't need checks for 1.2 - the initial version of openh264 that we
support is 1.3, so we only need checks for 1.4 and newer.
Ahh, didn't noticed this until checked the commit message of
8a3d9ca603f4d15ecaa9ca379cbaab4ecaec8ce4.
IMHO it seems to be better if we add this version check in the configure as
well.
(Did a quick verification with version modified in pkgconfig to 1.1.0,
configure is
still good for libopenh264 )
We don't need an explicit version check for 1.3 in configure - we require
that WelsGetCodecVersion can be found in the configure check, and that
function was added in 1.3 (explicitly for the purpose so that we can check
that the version that we have linked is the same as we compiled against).
Will send another fix for this if no against.
+#if OPENH264_VER_AT_LEAST(1, 4)
+{ "timestamp", "bit rate control based on timestamp",
0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE,
"rc_mode" },
+#endif
+
{ NULL }
};
This commit seems to lack something that actually sets the rc_mode value
in the parameters struct though - wasn't that part of the previous version
of the patch?
Did you mean setting rc_mode through parameters like bit_rate/qp?
This patch enables explicitly setting the rc_mode as part of that logic.
No, I didn't mean that.
The previous version of this patch had the following change:
-param.iRCMode= RC_QUALITY_MODE;
+param.iRCMode= s->rc_mode;
Now I don't find that part any longer in this patch - and without that
change, the rest of the commit is pretty pointless, no?
// 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 v5 3/5] lavc/libopenh264enc: add bit rate control select support
> From: Martin Storsjö
> Sent: Tuesday, April 28, 2020 18:31
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Cc: Fu, Linjie
> Subject: Re: [FFmpeg-devel] [PATCH v5 3/5] lavc/libopenh264enc: add bit
> rate control select support
>
> On Tue, 28 Apr 2020, Linjie Fu wrote:
>
> > RC_BITRATE_MODE:
> >set BITS_EXCEEDED to iCurrentBitsLevel and allows QP adjust
> >in RcCalculatePictureQp().
> >
> > RC_BUFFERBASED_MODE:
> >use buffer status to adjust the video quality, introduced in
> >release 1.2.
> >
> > RC_TIMESTAMP_MODE:
> >bit rate control based on timestamp, introduced in release 1.4.
> >
> > Default to use RC_QUALITY_MODE.
> >
> > Signed-off-by: Linjie Fu
> > ---
> > libavcodec/libopenh264enc.c | 15 +++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index 0951412..93d3de2 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -49,6 +49,9 @@ typedef struct SVCContext {
> > int skip_frames;
> > int skipped;
> > int cabac;
> > +
> > +// rate control mode
> > +int rc_mode;
> > } SVCContext;
> >
> > #define OFFSET(x) offsetof(SVCContext, x)
> > @@ -73,6 +76,18 @@ static const AVOption options[] = {
> > { "max_nal_size", "set maximum NAL size in bytes",
> OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
> > { "allow_skip_frames", "allow skipping frames to hit the target
> > bitrate",
> OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> > { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 1, VE },
> > +
> > +{ "rc_mode", "Select rate control mode", OFFSET(rc_mode),
> AV_OPT_TYPE_INT, { .i64 = RC_QUALITY_MODE }, RC_OFF_MODE,
> RC_TIMESTAMP_MODE, VE, "rc_mode" },
> > +{ "off", "bit rate control off",
> > 0,
> AV_OPT_TYPE_CONST, { .i64 = RC_OFF_MODE }, 0, 0, VE, "rc_mode" },
> > +{ "quality", "quality mode",
> > 0,
> AV_OPT_TYPE_CONST, { .i64 = RC_QUALITY_MODE }, 0, 0, VE, "rc_mode" },
> > +{ "bitrate", "bitrate mode",
> > 0,
> AV_OPT_TYPE_CONST, { .i64 = RC_BITRATE_MODE }, 0, 0, VE, "rc_mode" },
> > +#if OPENH264_VER_AT_LEAST(1, 2)
> > +{ "buffer","using buffer status to adjust the video quality
> > (no bitrate
> control)", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BUFFERBASED_MODE }, 0, 0,
> VE, "rc_mode" },
> > +#endif
>
> We don't need checks for 1.2 - the initial version of openh264 that we
> support is 1.3, so we only need checks for 1.4 and newer.
Ahh, didn't noticed this until checked the commit message of
8a3d9ca603f4d15ecaa9ca379cbaab4ecaec8ce4.
IMHO it seems to be better if we add this version check in the configure as
well.
(Did a quick verification with version modified in pkgconfig to 1.1.0,
configure is
still good for libopenh264 )
Will send another fix for this if no against.
> > +#if OPENH264_VER_AT_LEAST(1, 4)
> > +{ "timestamp", "bit rate control based on timestamp",
> 0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE,
> "rc_mode" },
> > +#endif
> > +
> > { NULL }
> > };
>
> This commit seems to lack something that actually sets the rc_mode value
> in the parameters struct though - wasn't that part of the previous version
> of the patch?
>
Did you mean setting rc_mode through parameters like bit_rate/qp?
This patch enables explicitly setting the rc_mode as part of that logic.
As to full enhancement, it's WIP and I'm still considering some parameters
like -qp to specify exact QP. (libopenh264 seems to only accept iMax/MinQp
in RC_QUALITY_MODE) For specific QP settings like -qp 26, we have to select
RC_OFF_MODE and specify QP by iDLayerQp in sSpatialLayers, which seems
a little bit confusing. (setting -qp 26 which leads to a non-quality mode)
- 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 v5 3/5] lavc/libopenh264enc: add bit rate control select support
On Tue, 28 Apr 2020, Linjie Fu wrote:
RC_BITRATE_MODE:
set BITS_EXCEEDED to iCurrentBitsLevel and allows QP adjust
in RcCalculatePictureQp().
RC_BUFFERBASED_MODE:
use buffer status to adjust the video quality, introduced in
release 1.2.
RC_TIMESTAMP_MODE:
bit rate control based on timestamp, introduced in release 1.4.
Default to use RC_QUALITY_MODE.
Signed-off-by: Linjie Fu
---
libavcodec/libopenh264enc.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 0951412..93d3de2 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -49,6 +49,9 @@ typedef struct SVCContext {
int skip_frames;
int skipped;
int cabac;
+
+// rate control mode
+int rc_mode;
} SVCContext;
#define OFFSET(x) offsetof(SVCContext, x)
@@ -73,6 +76,18 @@ static const AVOption options[] = {
{ "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size),
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
{ "allow_skip_frames", "allow skipping frames to hit the target bitrate",
OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
{ "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, 0,
1, VE },
+
+{ "rc_mode", "Select rate control mode", OFFSET(rc_mode), AV_OPT_TYPE_INT, { .i64 =
RC_QUALITY_MODE }, RC_OFF_MODE, RC_TIMESTAMP_MODE, VE, "rc_mode" },
+{ "off", "bit rate control off",
0, AV_OPT_TYPE_CONST, { .i64 = RC_OFF_MODE }, 0, 0, VE, "rc_mode" },
+{ "quality", "quality mode",
0, AV_OPT_TYPE_CONST, { .i64 = RC_QUALITY_MODE }, 0, 0, VE, "rc_mode" },
+{ "bitrate", "bitrate mode",
0, AV_OPT_TYPE_CONST, { .i64 = RC_BITRATE_MODE }, 0, 0, VE, "rc_mode" },
+#if OPENH264_VER_AT_LEAST(1, 2)
+{ "buffer","using buffer status to adjust the video quality (no bitrate
control)", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BUFFERBASED_MODE }, 0, 0, VE, "rc_mode" },
+#endif
We don't need checks for 1.2 - the initial version of openh264 that we
support is 1.3, so we only need checks for 1.4 and newer.
+#if OPENH264_VER_AT_LEAST(1, 4)
+{ "timestamp", "bit rate control based on timestamp",
0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE, "rc_mode" },
+#endif
+
{ NULL }
};
This commit seems to lack something that actually sets the rc_mode value
in the parameters struct though - wasn't that part of the previous version
of the patch?
// 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 v5 3/5] lavc/libopenh264enc: add bit rate control select support
RC_BITRATE_MODE:
set BITS_EXCEEDED to iCurrentBitsLevel and allows QP adjust
in RcCalculatePictureQp().
RC_BUFFERBASED_MODE:
use buffer status to adjust the video quality, introduced in
release 1.2.
RC_TIMESTAMP_MODE:
bit rate control based on timestamp, introduced in release 1.4.
Default to use RC_QUALITY_MODE.
Signed-off-by: Linjie Fu
---
libavcodec/libopenh264enc.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 0951412..93d3de2 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -49,6 +49,9 @@ typedef struct SVCContext {
int skip_frames;
int skipped;
int cabac;
+
+// rate control mode
+int rc_mode;
} SVCContext;
#define OFFSET(x) offsetof(SVCContext, x)
@@ -73,6 +76,18 @@ static const AVOption options[] = {
{ "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size),
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
{ "allow_skip_frames", "allow skipping frames to hit the target bitrate",
OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
{ "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 },
0, 1, VE },
+
+{ "rc_mode", "Select rate control mode", OFFSET(rc_mode), AV_OPT_TYPE_INT,
{ .i64 = RC_QUALITY_MODE }, RC_OFF_MODE, RC_TIMESTAMP_MODE, VE, "rc_mode" },
+{ "off", "bit rate control off",
0, AV_OPT_TYPE_CONST, { .i64 = RC_OFF_MODE }, 0, 0, VE,
"rc_mode" },
+{ "quality", "quality mode",
0, AV_OPT_TYPE_CONST, { .i64 = RC_QUALITY_MODE }, 0, 0, VE,
"rc_mode" },
+{ "bitrate", "bitrate mode",
0, AV_OPT_TYPE_CONST, { .i64 = RC_BITRATE_MODE }, 0, 0, VE,
"rc_mode" },
+#if OPENH264_VER_AT_LEAST(1, 2)
+{ "buffer","using buffer status to adjust the video quality (no
bitrate control)", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BUFFERBASED_MODE }, 0, 0,
VE, "rc_mode" },
+#endif
+#if OPENH264_VER_AT_LEAST(1, 4)
+{ "timestamp", "bit rate control based on timestamp",
0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE }, 0, 0, VE,
"rc_mode" },
+#endif
+
{ NULL }
};
--
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".
