Message ID | ff9b0b22be35733a7e3931b222bd643d00e67174.camel@amazon.it |
---|---|
State | New |
Headers | show |
Series | None | expand |
On Wed, 2023-06-21 at 15:57 +0000, Carotti, Elias wrote: > Hi all, > please find the second part of the patch set. > Best, > Elias Hi all, please find the second part of the patch, updating libavcodec/libx264.c to use the AVVideoHint side data. This should use the block_size field to scan the AVVideoRect array. Best, Elias NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
Quoting Carotti, Elias (2023-07-03 17:55:40) > On Wed, 2023-06-21 at 15:57 +0000, Carotti, Elias wrote: > > Hi all, > > please find the second part of the patch set. > > Best, > > Elias > > Hi all, > please find the second part of the patch, updating libavcodec/libx264.c > to use the AVVideoHint side data. This should use the block_size field > to scan the AVVideoRect array. > > Best, > Elias > > > > NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico > > > > From 5bdb624d8dbcff96493a63f02f68a3961cf72723 Mon Sep 17 00:00:00 2001 > From: Elias Carotti <eliascrt@amazon.it> > Date: Tue, 20 Jun 2023 19:29:08 +0200 > Subject: [PATCH 2/2] lavc/libx264: add mb_info option > > Pass the information about unchanged parts of the frame by means of the > AVVideoHint side data. > --- > Changelog | 1 + > doc/APIchanges | 3 ++ > libavcodec/libx264.c | 94 ++++++++++++++++++++++++++++++++++++++++++++ > libavcodec/version.h | 2 +- > 4 files changed, 99 insertions(+), 1 deletion(-) > > diff --git a/Changelog b/Changelog > index 3876082844..70b0fe94a3 100644 > --- a/Changelog > +++ b/Changelog > @@ -25,6 +25,7 @@ version <next>: > - Raw VVC bitstream parser, muxer and demuxer > - Bitstream filter for editing metadata in VVC streams > - Bitstream filter for converting VVC from MP4 to Annex B > +- support for the P_SKIP hinting to speed up libx264 encoding > > version 6.0: > - Radiance HDR image support > diff --git a/doc/APIchanges b/doc/APIchanges > index bfe04556d2..89ff88af15 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09 > > API changes, most recent first: > > +2023-06-21 - xxxxxxxxxx - lavc 60.23.100 - libx264.c > + Add mb_info option. Do we actually need the option? If the encoder's caller bothered with adding the side data, then I'd think it should always be used, otherwise why is it even there?
> From 5bdb624d8dbcff96493a63f02f68a3961cf72723 Mon Sep 17 00:00:00 2001 > From: Elias Carotti <eliascrt@amazon.it> > Date: Tue, 20 Jun 2023 19:29:08 +0200 > Subject: [PATCH 2/2] lavc/libx264: add mb_info option > > Pass the information about unchanged parts of the frame by means of > the AVVideoHint side data. > --- > Changelog | 1 + > doc/APIchanges | 3 ++ > libavcodec/libx264.c | 94 > ++++++++++++++++++++++++++++++++++++++++++++ > libavcodec/version.h | 2 +- > 4 files changed, 99 insertions(+), 1 deletion(-) > > diff --git a/Changelog b/Changelog > index 3876082844..70b0fe94a3 100644 > --- a/Changelog > +++ b/Changelog > @@ -25,6 +25,7 @@ version <next>: > - Raw VVC bitstream parser, muxer and demuxer > - Bitstream filter for editing metadata in VVC streams > - Bitstream filter for converting VVC from MP4 to Annex B > +- support for the P_SKIP hinting to speed up libx264 encoding > > version 6.0: > - Radiance HDR image support > diff --git a/doc/APIchanges b/doc/APIchanges index > bfe04556d2..89ff88af15 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -2,6 +2,9 @@ The last version increases of all libraries were on > 2023-02-09 > > API changes, most recent first: > > +2023-06-21 - xxxxxxxxxx - lavc 60.23.100 - libx264.c > + Add mb_info option. > Do we actually need the option? If the encoder's caller bothered with adding the side data, then I'd think it should always be used, otherwise why is it even there? I agree, I just thought I had to update that file as well, if that's not the case feel free to remove it. Best Elias NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
> +2023-06-21 - xxxxxxxxxx - lavc 60.23.100 - libx264.c > + Add mb_info option. > Do we actually need the option? If the encoder's caller bothered with adding the side data, then I'd think it should always be used, otherwise why is it even there? >I agree, I just thought I had to update that file as well, if that's not the case feel free to remove it. >Best >Elias On a second thought, though, I think I misinterpreted your mail and I wrongly thought you were referring only to the APIchanges file. The thing is that libx264 expects a boolean value (b_mb_info) to be set to check the mb_info. So, yes, in principle I'd agree with you but I think we still need to tell it in advance by setting the option because the encoder needs to track additional statistics. From x264.h: int b_mb_info; /* Use input mb_info data in x264_picture_t */ and then subsequent uses both in frame.h and analyse.c check for the flag to be set. Further on, this flag doesn't seem to be updated by the x264_encoder_reconfig(..) method, so the only possibility I see is setting it in advance in the setup phase. Best Elias NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
On date Monday 2023-07-03 15:55:40 +0000, Carotti, Elias wrote: > On Wed, 2023-06-21 at 15:57 +0000, Carotti, Elias wrote: > > Hi all, > > please find the second part of the patch set. > > Best, > > Elias > > Hi all, > please find the second part of the patch, updating libavcodec/libx264.c > to use the AVVideoHint side data. This should use the block_size field > to scan the AVVideoRect array. > > Best, > Elias > > > > NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico > > > From 5bdb624d8dbcff96493a63f02f68a3961cf72723 Mon Sep 17 00:00:00 2001 > From: Elias Carotti <eliascrt@amazon.it> > Date: Tue, 20 Jun 2023 19:29:08 +0200 > Subject: [PATCH 2/2] lavc/libx264: add mb_info option > > Pass the information about unchanged parts of the frame by means of the > AVVideoHint side data. > --- > Changelog | 1 + > doc/APIchanges | 3 ++ > libavcodec/libx264.c | 94 ++++++++++++++++++++++++++++++++++++++++++++ > libavcodec/version.h | 2 +- > 4 files changed, 99 insertions(+), 1 deletion(-) Rebased and applied, thanks.
From 2afd999f38fc3a7c3f56aba1d874de00bc9575ba Mon Sep 17 00:00:00 2001 From: Elias Carotti <eliascrt _at_ amazon _dot_ it> Date: Tue, 20 Jun 2023 19:29:08 +0200 Subject: [PATCH 2/2] lavc/libx264: add mb_info option Pass the information about unchanged parts of the frame by means of the AVVideoHint side data. --- Changelog | 1 + doc/APIchanges | 3 ++ libavcodec/libx264.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ libavcodec/version.h | 2 +- 4 files changed, 96 insertions(+), 1 deletion(-) diff --git a/Changelog b/Changelog index 9cf3df8d6f..ec1b848b04 100644 --- a/Changelog +++ b/Changelog @@ -21,6 +21,7 @@ version <next>: - Essential Video Coding frame merge bsf - bwdif_cuda filter - Microsoft RLE video encoder +- support for the P_SKIP hinting to speed up libx264 encoding version 6.0: - Radiance HDR image support diff --git a/doc/APIchanges b/doc/APIchanges index bfe04556d2..02723f5db7 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09 API changes, most recent first: +2023-06-21 - xxxxxxxxxx - lavc 60.22.100 - libx264.c + Add mb_info option. + 2023-06-21 - xxxxxxxxxx - lavu 58.14.100 - video_hint.h Add AVVideoHint API. diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index 5736f1efa7..c42e7ad39d 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -30,6 +30,7 @@ #include "libavutil/stereo3d.h" #include "libavutil/time.h" #include "libavutil/intreadwrite.h" +#include "libavutil/video_hint.h" #include "avcodec.h" #include "codec_internal.h" #include "encode.h" @@ -48,6 +49,9 @@ // from x264.h, for quant_offsets, Macroblocks are 16x16 // blocks of pixels (with respect to the luma plane) #define MB_SIZE 16 +#define MB_LSIZE 4 +#define MB_FLOOR(x) ((x) >> (MB_LSIZE)) +#define MB_CEIL(x) MB_FLOOR((x) + (MB_SIZE - 1)) typedef struct X264Opaque { #if FF_API_REORDERED_OPAQUE @@ -123,6 +127,8 @@ typedef struct X264Context { * encounter a frame with ROI side data. */ int roi_warned; + + int mb_info; } X264Context; static void X264_log(void *p, int level, const char *fmt, va_list args) @@ -295,6 +301,7 @@ static void free_picture(x264_picture_t *pic) av_free(pic->extra_sei.payloads[i].payload); av_freep(&pic->extra_sei.payloads); av_freep(&pic->prop.quant_offsets); + av_freep(&pic->prop.mb_info); pic->extra_sei.num_payloads = 0; } @@ -320,6 +327,74 @@ static enum AVPixelFormat csp_to_pixfmt(int csp) return AV_PIX_FMT_NONE; } +static void av_always_inline mbinfo_compute_changed_coords(const AVVideoRect *rect, + int *min_x, + int *max_x, + int *min_y, + int *max_y) +{ + *min_y = MB_FLOOR(rect->y); + *max_y = MB_CEIL(rect->y + rect->height); + *min_x = MB_FLOOR(rect->x); + *max_x = MB_CEIL(rect->x + rect->width); +} + +static void av_always_inline mbinfo_compute_constant_coords(const AVVideoRect *rect, + int *min_x, + int *max_x, + int *min_y, + int *max_y) +{ + *min_y = MB_CEIL(rect->y); + *max_y = MB_FLOOR(rect->y + rect->height); + *min_x = MB_CEIL(rect->x); + *max_x = MB_FLOOR(rect->x + rect->width); +} + +static int setup_mb_info(AVCodecContext *ctx, x264_picture_t *pic, + const AVFrame *frame, + const AVVideoHint *info) +{ + int mb_width = (frame->width + MB_SIZE - 1) / MB_SIZE; + int mb_height = (frame->height + MB_SIZE - 1) / MB_SIZE; + + const AVVideoRect *mbinfo_rects; + int nb_rects; + uint8_t *mbinfo; + + mbinfo_rects = (const AVVideoRect *)av_video_hint_rects(info); + nb_rects = info->nb_rects; + + mbinfo = av_calloc(mb_width * mb_height, sizeof(*mbinfo)); + if (!mbinfo) + return AVERROR(ENOMEM); + +#define COMPUTE_MBINFO(mbinfo_filler_, mbinfo_marker_, compute_coords_fn_) \ + memset(mbinfo, mbinfo_filler_, sizeof(*mbinfo) * mb_width * mb_height); \ + \ + for (int i = 0; i < nb_rects; i++) { \ + int min_x, max_x, min_y, max_y; \ + \ + compute_coords_fn_(mbinfo_rects, &min_x, &max_x, &min_y, &max_y); \ + for (int mb_y = min_y; mb_y < max_y; ++mb_y) { \ + memset(mbinfo + mb_y * mb_width + min_x, mbinfo_marker_, max_x - min_x); \ + } \ + \ + mbinfo_rects++; \ + } \ + + if (info->type == AV_VIDEO_HINT_CHANGED) { + COMPUTE_MBINFO(X264_MBINFO_CONSTANT, 0, mbinfo_compute_changed_coords); + } else /* if (info->type == AV_VIDEO_HINT_CHANGED) */ { + COMPUTE_MBINFO(0, X264_MBINFO_CONSTANT, mbinfo_compute_constant_coords); + } + + pic->prop.mb_info = mbinfo; + pic->prop.mb_info_free = av_free; + + return 0; +} + static int setup_roi(AVCodecContext *ctx, x264_picture_t *pic, int bit_depth, const AVFrame *frame, const uint8_t *data, size_t size) { @@ -404,6 +479,7 @@ static int setup_frame(AVCodecContext *ctx, const AVFrame *frame, int64_t wallclock = 0; int bit_depth, ret; AVFrameSideData *sd; + AVFrameSideData *mbinfo_sd; *ppic = NULL; if (!frame) @@ -499,6 +575,17 @@ FF_ENABLE_DEPRECATION_WARNINGS goto fail; } + mbinfo_sd = av_frame_get_side_data(frame, AV_FRAME_DATA_VIDEO_HINT); + if (mbinfo_sd) { + int ret = setup_mb_info(ctx, pic, frame, (const AVVideoHint *)mbinfo_sd->data); + if (ret < 0) { + /* No need to fail here, this is not fatal. We just proceed with no + * mb_info and log a message */ + + av_log(ctx, AV_LOG_WARNING, "setup_mb_info failed with error: %s\n", av_err2str(ret)); + } + } + if (x4->udu_sei) { for (int j = 0; j < frame->nb_side_data; j++) { AVFrameSideData *side_data = frame->side_data[j]; @@ -1102,6 +1189,9 @@ FF_ENABLE_DEPRECATION_WARNINGS } } + x4->params.analyse.b_mb_info = x4->mb_info; + x4->params.analyse.b_fast_pskip = 1; + // update AVCodecContext with x264 parameters avctx->has_b_frames = x4->params.i_bframe ? x4->params.i_bframe_pyramid ? 2 : 1 : 0; @@ -1311,6 +1401,7 @@ static const AVOption options[] = { { "noise_reduction", "Noise reduction", OFFSET(noise_reduction), AV_OPT_TYPE_INT, { .i64 = -1 }, INT_MIN, INT_MAX, VE }, { "udu_sei", "Use user data unregistered SEI if available", OFFSET(udu_sei), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, { "x264-params", "Override the x264 configuration using a :-separated list of key=value parameters", OFFSET(x264_params), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE }, + { "mb_info", "Set mb_info data through AVSideData, only useful when used from the API", OFFSET(mb_info), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, { NULL }, }; diff --git a/libavcodec/version.h b/libavcodec/version.h index da6f3a84ac..9411511e04 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -29,7 +29,7 @@ #include "version_major.h" -#define LIBAVCODEC_VERSION_MINOR 21 +#define LIBAVCODEC_VERSION_MINOR 22 #define LIBAVCODEC_VERSION_MICRO 100 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ -- 2.34.1