From patchwork Thu Aug 15 18:55:31 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Tomas_H=C3=A4rdin?= X-Patchwork-Id: 14526 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id ED35D4471D8 for ; Thu, 15 Aug 2019 21:55:40 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C55DC68ABDA; Thu, 15 Aug 2019 21:55:40 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail.acc.umu.se (mail.acc.umu.se [130.239.18.156]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6B37268AB76 for ; Thu, 15 Aug 2019 21:55:34 +0300 (EEST) Received: from localhost (localhost.localdomain [127.0.0.1]) by amavisd-new (Postfix) with ESMTP id D3AC644B90 for ; Thu, 15 Aug 2019 20:55:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=acc.umu.se; s=mail1; t=1565895333; bh=0d/HKpgnBq81YLaEyt/H4A+g+mY2zgdqCe6i21ZLc84=; h=Subject:From:To:Date:In-Reply-To:References:From; b=F5O+00SqWNzmplHgxXrtFK0kM7zt9sSSQtqiKydWVd3QOEf2km2+Y+UVm1yswudiX dDWvFqq1EYuD0iuqrv5XkC5iauwQbUonWCw1bUeDt/qMu7oYNYwK1z7KljOuNJ9eoc mqXdEEcydNZ3H6c4ryJa7HdMyN/2xK1/449qzRvNrH5gXpUIeZ7R7WU0vNH/ypoZ0y fldJO1NzeaAYvFJg3TGMGss5vGYxAh+3OiNFckgMNmX8ZeAauMeVGzBvH5G0p/CuLB Fwk9TO3fKb9O1iEvar61D0+SzerrjWTiQl/Nw25EWmgfzbzE9xd4GqxubL2EhnAAa3 RdJDcgE00nKrA== Received: from laptop.lan (h-39-105.A258.priv.bahnhof.se [79.136.39.105]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: tjoppen) by mail.acc.umu.se (Postfix) with ESMTPSA id 99CB444B8D for ; Thu, 15 Aug 2019 20:55:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=acc.umu.se; s=mail1; t=1565895332; bh=0d/HKpgnBq81YLaEyt/H4A+g+mY2zgdqCe6i21ZLc84=; h=Subject:From:To:Date:In-Reply-To:References:From; b=keI2GJS3o5+26f51SFCPVSI1N3XFxkmDlSgiwI3BzKl7Y/TWDHR9gPL7v3hwWuxGc zoV6+ITQrMn57rCYGud/9RY64FAQuFdzUea+cOIJtPGZaq/hfUWr26y8EGe+QknUtN Ma0dxzts0FY2P7Y86MNKMJAomCB5Gf2mEC5BHt7KCGWVStZwosmEwcz7wm2Fbi9hnp xuG3JglC+J+WYnNUU1zeNRzHLi7jd+/3nfWvJ0h1mv+VMRssk5hV58p4/rzv4dUMH0 kkedjJWjCfel4tDHtP3pi4nd538VHhqdjDwH8deSm9a1lNdaDe687sz+MBh4K6EGFD w1JSD2770N/LA== Message-ID: <8fbbdfbb2990d350c5c87cb1d23adf13eb21288e.camel@acc.umu.se> From: Tomas =?ISO-8859-1?Q?H=E4rdin?= To: FFmpeg development discussions and patches Date: Thu, 15 Aug 2019 20:55:31 +0200 In-Reply-To: <609e9021-c787-28a8-fd1c-3f88b55da97a@gmail.com> References: <4345eb71fc5a4f883301d27024fca7205fec2ec7.camel@acc.umu.se> <710af8be-3bb4-6a03-6381-a833334fab90@gmail.com> <609e9021-c787-28a8-fd1c-3f88b55da97a@gmail.com> User-Agent: Evolution 3.30.5-1.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH] cinepakenc: When rd_frame() doesn't use MC, mark pkt as keyframe and reset curframe X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" tor 2019-08-15 klockan 14:20 -0300 skrev James Almer: > On 8/15/2019 2:13 PM, Tomas Härdin wrote: > > tor 2019-08-15 klockan 12:54 -0300 skrev James Almer: > > > On 8/15/2019 11:34 AM, Tomas Härdin wrote: > > > > > > > > Aside: what is -keyint_min for exactly? It seems to be used as a max in > > > > msvideo1enc.c, but libx264.c and libxavs.c uses it and gop_size as > > > > minimum and maximum respectively.. Feels like the relevant encoders > > > > need to be brought into line (assuming it doesn't break user scripts) > > > > > > Afaik, keyint_min is minimum interval/distance between keyframes. So > > > yes, if it's being used as max distance then it's wrong, as that's what > > > gop_size is for. > > > > Make sense. But why is gop_size < keyint_min by default then? 12 vs 25. > > I have no idea. > > > I think the x264 people have complained multiple times about the > > default gop_size setting even > > The libx264 wrapper changes both to -1 in libavcodec, which means x264's > own defaults will be used instead. So if they complained then i guess it > was changed accordingly. Didn't know encoders could have their own defaults. Updated patch attached. I added some sanity checks on -keyint_min and -g as well. I couldn't find rl's contact information, hopefully they are subscribed to this list. /Tomas From 49d7ac02d35acbf3e30555eef215ae3a15c06684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= Date: Thu, 15 Aug 2019 16:20:23 +0200 Subject: [PATCH] cinepakenc: When rd_frame() doesn't use MC, mark pkt as keyframe and reset curframe This allows the encoder to make smarter choices for future frames, while still keeping keyframe distances within [keyint_min, gop_size]. Defaults and checks prevent nonsensical keyint_min / gop_size combinations. --- libavcodec/cinepakenc.c | 54 +++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/libavcodec/cinepakenc.c b/libavcodec/cinepakenc.c index 93917fafe8..396d3e5273 100644 --- a/libavcodec/cinepakenc.c +++ b/libavcodec/cinepakenc.c @@ -112,7 +112,7 @@ typedef struct CinepakEncContext { enum AVPixelFormat pix_fmt; int w, h; int frame_buf_size; - int curframe, keyint; + int curframe, keyint_min, keyint_max; AVLFG randctx; uint64_t lambda; int *codebook_input; @@ -168,6 +168,16 @@ static av_cold int cinepak_encode_init(AVCodecContext *avctx) return AVERROR(EINVAL); } + if (avctx->keyint_min <= 0) { + av_log(avctx, AV_LOG_ERROR, "-keyint_min must be >= 1\n"); + return AVERROR(EINVAL); + } + + if (avctx->keyint_min > avctx->gop_size) { + av_log(avctx, AV_LOG_ERROR, "-keyint_min > -g is not allowed. Did you mean -g?\n"); + return AVERROR(EINVAL); + } + if (!(s->last_frame = av_frame_alloc())) return AVERROR(ENOMEM); if (!(s->best_frame = av_frame_alloc())) @@ -213,7 +223,8 @@ static av_cold int cinepak_encode_init(AVCodecContext *avctx) s->h = avctx->height; s->frame_buf_size = frame_buf_size; s->curframe = 0; - s->keyint = avctx->keyint_min; + s->keyint_min = avctx->keyint_min; + s->keyint_max = avctx->gop_size; s->pix_fmt = avctx->pix_fmt; // set up AVFrames @@ -871,7 +882,7 @@ static int rd_strip(CinepakEncContext *s, int y, int h, int keyframe, uint8_t *last_data[4], int last_linesize[4], uint8_t *data[4], int linesize[4], uint8_t *scratch_data[4], int scratch_linesize[4], - unsigned char *buf, int64_t *best_score) + unsigned char *buf, int64_t *best_score, CinepakMode *best_mode) { int64_t score = 0; int best_size = 0; @@ -970,6 +981,7 @@ static int rd_strip(CinepakEncContext *s, int y, int h, int keyframe, if (best_size == 0 || score < *best_score) { *best_score = score; + *best_mode = mode; best_size = encode_mode(s, h, scratch_data, scratch_linesize, last_data, last_linesize, &info, @@ -1000,13 +1012,15 @@ static int write_cvid_header(CinepakEncContext *s, unsigned char *buf, } static int rd_frame(CinepakEncContext *s, const AVFrame *frame, - int isakeyframe, unsigned char *buf, int buf_size) + int isakeyframe, unsigned char *buf, int buf_size, + int *has_mc_out) { int num_strips, strip, i, y, nexty, size, temp_size, best_size; uint8_t *last_data [4], *data [4], *scratch_data [4]; int last_linesize[4], linesize[4], scratch_linesize[4]; int64_t best_score = 0, score, score_temp; int best_nstrips; + int has_mc; if (s->pix_fmt == AV_PIX_FMT_RGB24) { int x; @@ -1067,9 +1081,11 @@ static int rd_frame(CinepakEncContext *s, const AVFrame *frame, for (num_strips = s->min_strips; num_strips <= s->max_strips && num_strips <= s->h / MB_SIZE; num_strips++) { score = 0; size = 0; + has_mc = 0; for (y = 0, strip = 1; y < s->h; strip++, y = nexty) { int strip_height; + CinepakMode mode_temp; nexty = strip * s->h / num_strips; // <= s->h // make nexty the next multiple of 4 if not already there @@ -1101,16 +1117,21 @@ static int rd_frame(CinepakEncContext *s, const AVFrame *frame, last_data, last_linesize, data, linesize, scratch_data, scratch_linesize, s->frame_buf + size + CVID_HEADER_SIZE, - &score_temp)) < 0) + &score_temp, &mode_temp)) < 0) return temp_size; score += score_temp; size += temp_size; + + if (mode_temp == MODE_MC) { + has_mc = 1; + } } if (best_score == 0 || score < best_score) { best_score = score; - best_size = size + write_cvid_header(s, s->frame_buf, num_strips, size, isakeyframe); + *has_mc_out = has_mc; + best_size = size + write_cvid_header(s, s->frame_buf, num_strips, size, !has_mc); FFSWAP(AVFrame *, s->best_frame, s->scratch_frame); memcpy(buf, s->frame_buf, best_size); @@ -1152,21 +1173,25 @@ static int cinepak_encode_frame(AVCodecContext *avctx, AVPacket *pkt, const AVFrame *frame, int *got_packet) { CinepakEncContext *s = avctx->priv_data; - int ret; + int ret, has_mc; s->lambda = frame->quality ? frame->quality - 1 : 2 * FF_LAMBDA_SCALE; if ((ret = ff_alloc_packet2(avctx, pkt, s->frame_buf_size, 0)) < 0) return ret; - ret = rd_frame(s, frame, (s->curframe == 0), pkt->data, s->frame_buf_size); + ret = rd_frame(s, frame, (s->curframe == 0), pkt->data, s->frame_buf_size, &has_mc); pkt->size = ret; - if (s->curframe == 0) + if (s->curframe == 0 || (!has_mc && s->curframe + 1 >= s->keyint_min)) { + //if the packet doesn't use MODE_MC then we + //mark it as a keyframe and reset curframe + s->curframe = 0; pkt->flags |= AV_PKT_FLAG_KEY; + } *got_packet = 1; FFSWAP(AVFrame *, s->last_frame, s->best_frame); - if (++s->curframe >= s->keyint) + if (++s->curframe >= s->keyint_max) s->curframe = 0; return 0; @@ -1194,6 +1219,14 @@ static av_cold int cinepak_encode_end(AVCodecContext *avctx) return 0; } +static const AVCodecDefault cinepak_defaults[] = { + //-keyint_min 1 means any frame can be a keyframe, + //2 means there's at least one non-keyframe inbetween, and so on + { "keyint_min", "1" }, + { "g", "25" }, + { NULL }, +}; + AVCodec ff_cinepak_encoder = { .name = "cinepak", .long_name = NULL_IF_CONFIG_SMALL("Cinepak"), @@ -1205,4 +1238,5 @@ AVCodec ff_cinepak_encoder = { .close = cinepak_encode_end, .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_RGB24, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NONE }, .priv_class = &cinepak_class, + .defaults = cinepak_defaults, }; -- 2.20.1