| Message ID | 1586599419-21122-6-git-send-email-linjie.fu@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | patchset for the enhancement of libopenh264 encoder | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Quoting Linjie Fu (2020-04-11 12:03:35) > "slice mode" seems to be unnecessary since it could be determined by > -slices/max_nal_size. > > default: SM_FIXEDSLCNUM_SLICE mode with cpu-number slices. > -slices N: SM_FIXEDSLCNUM_SLICE mode with N slices. > -max_nal_size: SM_SIZELIMITED_SLICE mode with limited size slices. > > Add OPENH264_API_SLICE_MODE to remove this option after > LIBAVCODEC_VERSION_MAJOR = 59. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/libopenh264enc.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c > index e556f03..3a642f0 100644 > --- a/libavcodec/libopenh264enc.c > +++ b/libavcodec/libopenh264enc.c > @@ -33,6 +33,10 @@ > #include "internal.h" > #include "libopenh264.h" > > +#ifndef OPENH264_API_SLICE_MODE > +#define OPENH264_API_SLICE_MODE (LIBAVCODEC_VERSION_MAJOR < 59) > +#endif This should be moved to libavcodec/version.h with an FF_ prefix, so that it's not overlooked when we bump the version. Otherwise looks ok
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Anton Khirnov > Sent: Tuesday, April 14, 2020 19:11 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v3 5/9] lavc/libopenh264enc: set > slice_mode option to deprecated > > Quoting Linjie Fu (2020-04-11 12:03:35) > > "slice mode" seems to be unnecessary since it could be determined by > > -slices/max_nal_size. > > > > default: SM_FIXEDSLCNUM_SLICE mode with cpu-number slices. > > -slices N: SM_FIXEDSLCNUM_SLICE mode with N slices. > > -max_nal_size: SM_SIZELIMITED_SLICE mode with limited size slices. > > > > Add OPENH264_API_SLICE_MODE to remove this option after > > LIBAVCODEC_VERSION_MAJOR = 59. > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > libavcodec/libopenh264enc.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c > > index e556f03..3a642f0 100644 > > --- a/libavcodec/libopenh264enc.c > > +++ b/libavcodec/libopenh264enc.c > > @@ -33,6 +33,10 @@ > > #include "internal.h" > > #include "libopenh264.h" > > > > +#ifndef OPENH264_API_SLICE_MODE > > +#define OPENH264_API_SLICE_MODE > (LIBAVCODEC_VERSION_MAJOR < 59) > > +#endif > > This should be moved to libavcodec/version.h with an FF_ prefix, so that > it's not overlooked when we bump the version. Moved the macro to version.h and resend the whole patch set as V4. https://patchwork.ffmpeg.org/project/ffmpeg/patch/1586926427-20170-6-git-send-email-linjie.fu@intel.com/ https://patchwork.ffmpeg.org/project/ffmpeg/patch/1586926427-20170-10-git-send-email-linjie.fu@intel.com/ > Otherwise looks ok Thanks for the review. - Linjie
diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c index e556f03..3a642f0 100644 --- a/libavcodec/libopenh264enc.c +++ b/libavcodec/libopenh264enc.c @@ -33,6 +33,10 @@ #include "internal.h" #include "libopenh264.h" +#ifndef OPENH264_API_SLICE_MODE +#define OPENH264_API_SLICE_MODE (LIBAVCODEC_VERSION_MAJOR < 59) +#endif + #if !OPENH264_VER_AT_LEAST(1, 6) #define SM_SIZELIMITED_SLICE SM_DYN_SLICE #endif @@ -40,7 +44,9 @@ typedef struct SVCContext { const AVClass *av_class; ISVCEncoder *encoder; - int slice_mode; +#if OPENH264_API_SLICE_MODE + int slice_mode; // deprecated +#endif int loopfilter; char *profile; int max_nal_size; @@ -54,11 +60,13 @@ typedef struct SVCContext { #define OFFSET(x) offsetof(SVCContext, x) #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM +#define DEPRECATED AV_OPT_FLAG_DEPRECATED static const AVOption options[] = { +#if OPENH264_API_SLICE_MODE #if OPENH264_VER_AT_LEAST(1, 6) - { "slice_mode", "set slice mode", OFFSET(slice_mode), AV_OPT_TYPE_INT, { .i64 = SM_FIXEDSLCNUM_SLICE }, SM_SINGLE_SLICE, SM_RESERVED, VE, "slice_mode" }, + { "slice_mode", "set slice mode, use slices/max_nal_size", OFFSET(slice_mode), AV_OPT_TYPE_INT, { .i64 = SM_FIXEDSLCNUM_SLICE }, SM_SINGLE_SLICE, SM_RESERVED, VE|DEPRECATED, "slice_mode" }, #else - { "slice_mode", "set slice mode", OFFSET(slice_mode), AV_OPT_TYPE_INT, { .i64 = SM_AUTO_SLICE }, SM_SINGLE_SLICE, SM_RESERVED, VE, "slice_mode" }, + { "slice_mode", "set slice mode, use slices/max_nal_size", OFFSET(slice_mode), AV_OPT_TYPE_INT, { .i64 = SM_AUTO_SLICE }, SM_SINGLE_SLICE, SM_RESERVED, VE|DEPRECATED, "slice_mode" }, #endif { "fixed", "a fixed number of slices", 0, AV_OPT_TYPE_CONST, { .i64 = SM_FIXEDSLCNUM_SLICE }, 0, 0, VE, "slice_mode" }, #if OPENH264_VER_AT_LEAST(1, 6) @@ -69,6 +77,7 @@ static const AVOption options[] = { { "auto", "automatic number of slices according to number of threads", 0, AV_OPT_TYPE_CONST, { .i64 = SM_AUTO_SLICE }, 0, 0, VE, "slice_mode" }, { "dyn", "Dynamic slicing", 0, AV_OPT_TYPE_CONST, { .i64 = SM_DYN_SLICE }, 0, 0, VE, "slice_mode" }, #endif +#endif { "loopfilter", "enable loop filter", OFFSET(loopfilter), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, VE }, { "profile", "set profile restrictions", OFFSET(profile), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE }, { "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
"slice mode" seems to be unnecessary since it could be determined by -slices/max_nal_size. default: SM_FIXEDSLCNUM_SLICE mode with cpu-number slices. -slices N: SM_FIXEDSLCNUM_SLICE mode with N slices. -max_nal_size: SM_SIZELIMITED_SLICE mode with limited size slices. Add OPENH264_API_SLICE_MODE to remove this option after LIBAVCODEC_VERSION_MAJOR = 59. Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- libavcodec/libopenh264enc.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)