[FFmpeg-devel] [WIP] GSoC P frame support for FFV1 codec
Michael Niedermayer
michael at niedermayer.cc
Mon Mar 28 23:53:08 CEST 2016
Hi
On Mon, Mar 28, 2016 at 08:36:17PM +0300, Станислав Долганов wrote:
> Fix problems with multithread runs.
>
> --
> Stanislav Dolganov
> libavcodec/ffv1.c | 13 ++++
> libavcodec/ffv1.h | 4 +
> libavcodec/ffv1dec.c | 76 +++++++++++++++++++++++++
> libavcodec/ffv1enc.c | 87 +++++++++++++++++++++++++++++
> tests/ref/vsynth/vsynth1-ffv1 | 4 -
> tests/ref/vsynth/vsynth1-ffv1-v0 | 4 -
> tests/ref/vsynth/vsynth1-ffv1-v3-bgr0 | 4 -
> tests/ref/vsynth/vsynth1-ffv1-v3-yuv420p | 4 -
> tests/ref/vsynth/vsynth1-ffv1-v3-yuv422p10 | 4 -
> tests/ref/vsynth/vsynth1-ffv1-v3-yuv444p16 | 4 -
> tests/ref/vsynth/vsynth2-ffv1 | 4 -
> tests/ref/vsynth/vsynth2-ffv1-v0 | 4 -
> tests/ref/vsynth/vsynth2-ffv1-v3-bgr0 | 4 -
> tests/ref/vsynth/vsynth2-ffv1-v3-yuv420p | 4 -
> tests/ref/vsynth/vsynth2-ffv1-v3-yuv422p10 | 4 -
> tests/ref/vsynth/vsynth2-ffv1-v3-yuv444p16 | 4 -
> tests/ref/vsynth/vsynth3-ffv1 | 4 -
> tests/ref/vsynth/vsynth3-ffv1-v0 | 4 -
> tests/ref/vsynth/vsynth3-ffv1-v3-bgr0 | 4 -
> tests/ref/vsynth/vsynth3-ffv1-v3-yuv420p | 4 -
> tests/ref/vsynth/vsynth3-ffv1-v3-yuv422p10 | 4 -
> tests/ref/vsynth/vsynth3-ffv1-v3-yuv444p16 | 4 -
> 22 files changed, 214 insertions(+), 38 deletions(-)
> 5384aa4e0f22b0e53e986b31a4687ca19a7eb20c 0001-simple-P-frame-support.patch
> From 504dfc78085163d588b3f06d9e62c4d85ceebb17 Mon Sep 17 00:00:00 2001
> From: Stanislav Dolganov <dolganov at qst.hk>
> Date: Thu, 24 Mar 2016 13:53:43 +0300
> Subject: [PATCH 1/4] simple P frame support
>
> ---
> libavcodec/ffv1.c | 13 ++++-
> libavcodec/ffv1.h | 4 +-
> libavcodec/ffv1dec.c | 76 ++++++++++++++++++++++++++
> libavcodec/ffv1enc.c | 87 ++++++++++++++++++++++++++++++
> tests/ref/vsynth/vsynth1-ffv1 | 4 +-
> tests/ref/vsynth/vsynth1-ffv1-v0 | 4 +-
> tests/ref/vsynth/vsynth1-ffv1-v3-bgr0 | 4 +-
> tests/ref/vsynth/vsynth1-ffv1-v3-yuv420p | 4 +-
> tests/ref/vsynth/vsynth1-ffv1-v3-yuv422p10 | 4 +-
> tests/ref/vsynth/vsynth1-ffv1-v3-yuv444p16 | 4 +-
> tests/ref/vsynth/vsynth2-ffv1 | 4 +-
> tests/ref/vsynth/vsynth2-ffv1-v0 | 4 +-
> tests/ref/vsynth/vsynth2-ffv1-v3-bgr0 | 4 +-
> tests/ref/vsynth/vsynth2-ffv1-v3-yuv420p | 4 +-
> tests/ref/vsynth/vsynth2-ffv1-v3-yuv422p10 | 4 +-
> tests/ref/vsynth/vsynth2-ffv1-v3-yuv444p16 | 4 +-
> tests/ref/vsynth/vsynth3-ffv1 | 4 +-
> tests/ref/vsynth/vsynth3-ffv1-v0 | 4 +-
> tests/ref/vsynth/vsynth3-ffv1-v3-bgr0 | 4 +-
> tests/ref/vsynth/vsynth3-ffv1-v3-yuv420p | 4 +-
> tests/ref/vsynth/vsynth3-ffv1-v3-yuv422p10 | 4 +-
> tests/ref/vsynth/vsynth3-ffv1-v3-yuv444p16 | 4 +-
> 22 files changed, 214 insertions(+), 38 deletions(-)
>
> diff --git a/libavcodec/ffv1.c b/libavcodec/ffv1.c
> index 537409e..428bc8d 100644
> --- a/libavcodec/ffv1.c
> +++ b/libavcodec/ffv1.c
> @@ -51,12 +51,16 @@ av_cold int ff_ffv1_common_init(AVCodecContext *avctx)
>
> s->picture.f = av_frame_alloc();
> s->last_picture.f = av_frame_alloc();
> - if (!s->picture.f || !s->last_picture.f)
> + s->residual.f = av_frame_alloc();
> + if (!s->picture.f || !s->last_picture.f || !s->residual.f)
> return AVERROR(ENOMEM);
>
> s->width = avctx->width;
> s->height = avctx->height;
>
> + s->c_image_line_buf = av_mallocz_array(sizeof(*s->c_image_line_buf), s->width);
> + s->p_image_line_buf = av_mallocz_array(sizeof(*s->p_image_line_buf), s->width);
> +
> // defaults
> s->num_h_slices = 1;
> s->num_v_slices = 1;
> @@ -215,6 +219,10 @@ av_cold int ff_ffv1_close(AVCodecContext *avctx)
> ff_thread_release_buffer(avctx, &s->last_picture);
> av_frame_free(&s->last_picture.f);
>
> + if (s->residual.f)
> + ff_thread_release_buffer(avctx, &s->residual);
> + av_frame_free(&s->residual.f);
> +
> for (j = 0; j < s->max_slice_count; j++) {
> FFV1Context *fs = s->slice_context[j];
> for (i = 0; i < s->plane_count; i++) {
> @@ -226,6 +234,9 @@ av_cold int ff_ffv1_close(AVCodecContext *avctx)
> av_freep(&fs->sample_buffer);
> }
>
> + av_freep(&s->c_image_line_buf);
> + av_freep(&s->p_image_line_buf);
> +
> av_freep(&avctx->stats_out);
> for (j = 0; j < s->quant_table_count; j++) {
> av_freep(&s->initial_states[j]);
> diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
> index d9398e5..8d1f74a 100644
> --- a/libavcodec/ffv1.h
> +++ b/libavcodec/ffv1.h
> @@ -93,7 +93,7 @@ typedef struct FFV1Context {
> int flags;
> int picture_number;
> int key_frame;
> - ThreadFrame picture, last_picture;
> + ThreadFrame picture, last_picture, residual;
> struct FFV1Context *fsrc;
>
> AVFrame *cur;
> @@ -110,6 +110,8 @@ typedef struct FFV1Context {
> int colorspace;
> int16_t *sample_buffer;
>
> + uint16_t *p_image_line_buf, *c_image_line_buf;
> +
> int ec;
> int intra;
> int slice_damaged;
> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> index d2bf3a8..32e8d9b 100644
> --- a/libavcodec/ffv1dec.c
> +++ b/libavcodec/ffv1dec.c
> @@ -39,6 +39,74 @@
> #include "mathops.h"
> #include "ffv1.h"
>
> +static int ff_predict_frame(AVCodecContext *c, FFV1Context *f)
> +{
> + int ret, i, x, y;
> + AVFrame *curr = f->picture.f;
> + AVFrame *prev = f->last_picture.f;
> + AVFrame *residual = f->residual.f;
> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(prev->format);
> + int width = f->width;
> + int height = f->height;
> + int has_plane[4] = { 0 };
> + const int cw = AV_CEIL_RSHIFT(width, desc->log2_chroma_w);
> + const int ch = AV_CEIL_RSHIFT(height, desc->log2_chroma_h);
> +
> + if (f->residual.f)
> + av_frame_unref(f->residual.f);
> + if ((ret = av_frame_ref(f->residual.f, f->picture.f)) < 0)
> + return ret;
> + if ((ret = av_frame_make_writable(f->residual.f)) < 0) {
> + av_frame_unref(f->residual.f);
> + return ret;
> + }
> +
> + for (i = 0; i < desc->nb_components; i++)
> + has_plane[desc->comp[i].plane] = 1;
> +
> + for (i = 0; i < desc->nb_components && has_plane[i]; i++)
> + memset(residual->buf[i]->data, 0, residual->buf[i]->size * sizeof(*residual->buf[i]->data));
> +
> + for (i = 0; i < desc->nb_components; i++) {
> + const int w1 = (i == 1 || i == 2) ? cw : width;
> + const int h1 = (i == 1 || i == 2) ? ch : height;
> +
> + for (y = 0; y < h1; y++) {
> + memset(f->p_image_line_buf, 0, width * sizeof(*f->p_image_line_buf));
> + memset(f->c_image_line_buf, 0, width * sizeof(*f->c_image_line_buf));
> + av_read_image_line(f->c_image_line_buf,
> + (void *)curr->data,
> + curr->linesize,
> + desc,
> + 0, y, i, w1, 0);
> + av_read_image_line(f->p_image_line_buf,
> + (void *)prev->data,
> + prev->linesize,
> + desc,
> + 0, y, i, w1, 0);
> + for (x = 0; x < w1; ++x)
> + f->c_image_line_buf[x] ^= f->p_image_line_buf[x];
> + av_write_image_line(f->c_image_line_buf,
> + residual->data,
> + residual->linesize,
> + desc,
> + 0, y, i, w1);
> + memset(f->p_image_line_buf, 0, width * sizeof(*f->p_image_line_buf));
> + av_read_image_line(f->p_image_line_buf,
> + (void *)residual->data,
> + residual->linesize,
> + desc,
> + 0, y, i, w1, 0);
> + av_assert0(!memcmp(f->p_image_line_buf, f->c_image_line_buf, width * sizeof(*f->c_image_line_buf)));
> + }
> + }
> +
> + if ((ret = av_frame_ref(f->picture.f, f->residual.f)) < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> static inline av_flatten int get_symbol_inline(RangeCoder *c, uint8_t *state,
> int is_signed)
> {
> @@ -935,6 +1003,8 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
> p->key_frame = 0;
> }
>
> + p->pict_type = p->key_frame ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
> +
> if ((ret = ff_thread_get_buffer(avctx, &f->picture, AV_GET_BUFFER_FLAG_REF)) < 0)
> return ret;
>
> @@ -1022,9 +1092,15 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
>
> f->picture_number++;
>
> + if (!p->key_frame) {
> + if ((ret = ff_predict_frame(avctx, f)) < 0)
> + return ret;
> + }
this would break decoding files that where generated before the
patchset
> +
> if (f->last_picture.f)
> ff_thread_release_buffer(avctx, &f->last_picture);
> f->cur = NULL;
> +
> if ((ret = av_frame_ref(data, f->picture.f)) < 0)
> return ret;
there should be no unrelated changes in the patch and you might
want to stash the patches into one
spliting a patche into functional seperate parts makes sense but
a implementation with bugs and seperate bugfixes dont make much
sense.
[...]
> ffv1dec.c | 23 ++++++++++++-----------
> ffv1enc.c | 1 -
> 2 files changed, 12 insertions(+), 12 deletions(-)
> dfab91f851d49176f38ce234f781a5f8a5add1bf 0004-ffv1-threads-bug-fix.patch
> From 0807aa141fdf700434816bd37368d92b57a58feb Mon Sep 17 00:00:00 2001
> From: Stanislav Dolganov <dolganov at qst.hk>
> Date: Mon, 28 Mar 2016 20:17:07 +0300
> Subject: [PATCH 4/4] ffv1 threads bug fix
>
> ---
> libavcodec/ffv1dec.c | 23 ++++++++++++-----------
> libavcodec/ffv1enc.c | 1 -
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> index b1aaeea..08c087a 100644
> --- a/libavcodec/ffv1dec.c
> +++ b/libavcodec/ffv1dec.c
> @@ -39,7 +39,7 @@
> #include "mathops.h"
> #include "ffv1.h"
>
> -static int ff_predict_frame(AVCodecContext *c, FFV1Context *f)
> +static int ff_predict_frame(AVCodecContext *avctx, FFV1Context *f)
> {
> int ret, i, x, y;
> AVFrame *curr = f->picture.f;
> @@ -53,11 +53,11 @@ static int ff_predict_frame(AVCodecContext *c, FFV1Context *f)
> const int ch = AV_CEIL_RSHIFT(height, desc->log2_chroma_h);
>
> if (f->residual.f)
> - av_frame_unref(f->residual.f);
> - if ((ret = av_frame_ref(f->residual.f, f->picture.f)) < 0)
> + ff_thread_release_buffer(avctx, &f->residual);
> + if ((ret = ff_thread_ref_frame(&f->residual, &f->picture)) < 0)
> return ret;
> if ((ret = av_frame_make_writable(f->residual.f)) < 0) {
> - av_frame_unref(f->residual.f);
> + ff_thread_release_buffer(avctx, &f->residual);
> return ret;
> }
>
> @@ -88,7 +88,6 @@ static int ff_predict_frame(AVCodecContext *c, FFV1Context *f)
> desc,
> 0, y, i, w1, 0);
> for (x = 0; x < w1; ++x)
> - //f->c_image_line_buf[x] ^= f->p_image_line_buf[x];
> f->c_image_line_buf[x] = (f->c_image_line_buf[x] + f->p_image_line_buf[x] - (max_val >> 2)) & (max_val - 1);
> av_write_image_line(f->c_image_line_buf,
> residual->data,
> @@ -105,8 +104,10 @@ static int ff_predict_frame(AVCodecContext *c, FFV1Context *f)
> }
> }
>
> - if ((ret = av_frame_ref(f->picture.f, f->residual.f)) < 0)
> - return ret;
> + /*if ((ret = av_frame_ref(f->picture.f, f->residual.f)) < 0)
> + return ret;*/
there should be no outcommented pieces of code added
[...]
thanks
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is what and why we do it that matters, not just one of them.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160328/6913d63c/attachment.sig>
More information about the ffmpeg-devel
mailing list