[FFmpeg-devel] [GSOC][PATCH 1/4] factoring obmc out of snow
Станислав Долганов
stanislav.dolganov at gmail.com
Mon Aug 29 23:00:41 EEST 2016
> i mentioned this previously
> the checks should be removed from the inner loop
> either by having seperate loops or allocating large enough arrays
It was in the previous version and it's better to update in later patch,
though.
> Why is the assert disabled ?
It moved to SnowEncoder code while initialization cause the block_state
variable doesn't belong to OBMC.
More fixes are in the attached patch.
2016-08-21 15:51 GMT+01:00 Michael Niedermayer <michael at niedermayer.cc>:
> On Sun, Aug 21, 2016 at 09:32:31AM +0300, Станислав Долганов wrote:
> > Sorry, I forgot to return ok from encode init. Here updated patch.
> >
> > 2016-08-20 23:44 GMT+03:00 Michael Niedermayer <michael at niedermayer.cc>:
> >
> > > On Sat, Aug 20, 2016 at 11:01:29PM +0300, Станислав Долганов wrote:
> > > > Fixed it.
> > >
> > > breaks fate (this worked with the previous patch)
> > >
> > > make: *** [fate-vsynth1-snow] Error 139
> > > make: *** [fate-vsynth1-snow-hpel] Error 139
> > > make: *** [fate-vsynth1-snow-ll] Error 139
> > > make: *** [fate-filter-mcdeint-fast] Error 139
> > > make: *** [fate-filter-mcdeint-medium] Error 139
> > > make: *** [fate-vsynth2-snow] Error 139
> > > make: *** [fate-vsynth2-snow-hpel] Error 139
> > > make: *** [fate-vsynth2-snow-ll] Error 139
> > > make: *** [fate-vsynth_lena-snow] Error 139
> > > make: *** [fate-vsynth_lena-snow-hpel] Error 139
> > > make: *** [fate-vsynth_lena-snow-ll] Error 139
> > >
> > > [...]
> > >
> > >
> > > --
> > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC7
> 87040B0FAB
> > >
> > > Modern terrorism, a quick summary: Need oil, start war with country
> that
> > > has oil, kill hundread thousand in war. Let country fall into chaos,
> > > be surprised about raise of fundamantalists. Drop more bombs, kill more
> > > people, be surprised about them taking revenge and drop even more bombs
> > > and strip your own citizens of their rights and freedoms. to be
> continued
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > >
> >
> >
> > --
> > Станислав Долганов
>
> > b/libavcodec/Makefile | 8
> > b/libavcodec/obmc.c | 61 +
> > b/libavcodec/obmc.h | 30
> > b/libavcodec/obme.c | 1131 ++++++++++++++++++++++++++++++++++++
> > b/libavcodec/obme.h | 33 +
> > b/libavcodec/obmemc.c | 652 ++++++++++++++++++++
> > b/libavcodec/obmemc.h | 522 ++++++++++++++++
> > b/libavcodec/obmemc_data.h | 132 ++++
> > b/libavcodec/snow.c | 571 ------------------
> > b/libavcodec/snow.h | 356 -----------
> > b/libavcodec/snowdec.c | 235 ++-----
> > b/libavcodec/snowenc.c | 1408 +++++++-----------------------
> ---------------
> > libavcodec/snowdata.h | 132 ----
> > 13 files changed, 2896 insertions(+), 2375 deletions(-)
> > 7fd1c474b26a198cac3d5a85ef27ef1acc47c80b 0001-factoring-obmc-out-of-
> snow.patch
> > From 936143b88dbbe666d2c9be38fe37f715c3e91e6b Mon Sep 17 00:00:00 2001
> > From: Stanislav Dolganov <dolganov at qst.hk>
> > Date: Thu, 18 Aug 2016 14:07:35 +0300
> > Subject: [PATCH 1/4] factoring obmc out of snow
> >
> > ---
> > libavcodec/Makefile | 8 +-
> > libavcodec/obmc.c | 61 ++
> > libavcodec/obmc.h | 30 +
> > libavcodec/obme.c | 1131 +++++++++++++++++++++++++++++++++++++
> > libavcodec/obme.h | 33 ++
> > libavcodec/obmemc.c | 652 +++++++++++++++++++++
> > libavcodec/obmemc.h | 522 +++++++++++++++++
> > libavcodec/obmemc_data.h | 132 +++++
> > libavcodec/snow.c | 571 +------------------
> > libavcodec/snow.h | 356 +-----------
> > libavcodec/snowdata.h | 132 -----
> > libavcodec/snowdec.c | 235 +++-----
> > libavcodec/snowenc.c | 1408 ++++++++----------------------
> ----------------
> > 13 files changed, 2896 insertions(+), 2375 deletions(-)
> > create mode 100644 libavcodec/obmc.c
> > create mode 100644 libavcodec/obmc.h
> > create mode 100644 libavcodec/obme.c
> > create mode 100644 libavcodec/obme.h
> > create mode 100644 libavcodec/obmemc.c
> > create mode 100644 libavcodec/obmemc.h
> > create mode 100644 libavcodec/obmemc_data.h
> > delete mode 100644 libavcodec/snowdata.h
> >
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index b375720..f24cd81 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
>
> > @@ -511,9 +511,11 @@ OBJS-$(CONFIG_SMACKAUD_DECODER) += smacker.o
> > OBJS-$(CONFIG_SMACKER_DECODER) += smacker.o
> > OBJS-$(CONFIG_SMC_DECODER) += smc.o
> > OBJS-$(CONFIG_SMVJPEG_DECODER) += smvjpegdec.o
> > -OBJS-$(CONFIG_SNOW_DECODER) += snowdec.o snow.o snow_dwt.o
> > -OBJS-$(CONFIG_SNOW_ENCODER) += snowenc.o snow.o snow_dwt.o
> \
> > - h263.o ituh263enc.o
> > +OBJS-$(CONFIG_SNOW_DECODER) += snowdec.o snow.o snow_dwt.o\
> > +
> obmemc.o obmc.o
>
> inconsistent indention and tabs
>
>
> > +OBJS-$(CONFIG_SNOW_ENCODER) += snowenc.o snow.o snow_dwt.o\
> > + h263.o ituh263enc.o\
> > + obmemc.o obme.o
> > OBJS-$(CONFIG_SOL_DPCM_DECODER) += dpcm.o
> > OBJS-$(CONFIG_SONIC_DECODER) += sonic.o
> > OBJS-$(CONFIG_SONIC_ENCODER) += sonic.o
> > diff --git a/libavcodec/obmc.c b/libavcodec/obmc.c
> > new file mode 100644
> > index 0000000..10a3afe
> > --- /dev/null
> > +++ b/libavcodec/obmc.c
> > @@ -0,0 +1,61 @@
> > +/*
> > + * Copyright (C) 2004 Michael Niedermayer <michaelni at gmx.at>
> > + * Copyright (C) 2006 Robert Edele <yartrebo at earthlink.net>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
> > + #include "obmc.h"
> > +
> > +int ff_obmc_decode_init(OBMCContext *f) {
> > + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(f->avctx->
> pix_fmt);
> > + if (!desc)
> > + return AVERROR_INVALIDDATA;
> > + int i;
> > + f->nb_planes = 0;
> > + for (i = 0; i < desc->nb_components; i++)
> > + f->nb_planes = FFMAX(f->nb_planes, desc->comp[i].plane + 1);
> > +
> > + avcodec_get_chroma_sub_sample(f->avctx->pix_fmt,
> &f->chroma_h_shift, &f->chroma_v_shift);
> > +
> > + return 0;
> > +}
> > +
> > +int ff_obmc_predecode_frame(OBMCContext *f) {
> > + int plane_index, ret;
> > + for(plane_index=0; plane_index < f->nb_planes; plane_index++){
> > + PlaneObmc *pc= &f->plane[plane_index];
> > + pc->fast_mc= pc->diag_mc && pc->htaps==6 && pc->hcoeff[0]==40
> > + && pc->hcoeff[1]==-10
> > + && pc->hcoeff[2]==2;
> > + }
> > +
>
> > + ff_obmc_alloc_blocks(f);
>
> missing return code check
>
>
> [...]
> > diff --git a/libavcodec/obmc.h b/libavcodec/obmc.h
> > new file mode 100644
> > index 0000000..019ffe2
> > --- /dev/null
> > +++ b/libavcodec/obmc.h
> > @@ -0,0 +1,30 @@
> > +/*
> > + * Copyright (C) 2004 Michael Niedermayer <michaelni at gmx.at>
> > + * Copyright (C) 2006 Robert Edele <yartrebo at earthlink.net>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
> > +#ifndef AVCODEC_OBMC_H
> > +#define AVCODEC_OBMC_H
> > +
> > +#include "obmemc.h"
> > +
>
> > +int ff_obmc_decode_init(OBMCContext *f);
> > +int ff_obmc_predecode_frame(OBMCContext *f);
>
> Missing doxy documentation for non static functions
>
>
> > +
> > +#endif /* AVCODEC_OBMC_H */
> > diff --git a/libavcodec/obme.c b/libavcodec/obme.c
> > new file mode 100644
> > index 0000000..b7edaca
> > --- /dev/null
> > +++ b/libavcodec/obme.c
> > @@ -0,0 +1,1131 @@
> > +/*
> > + * Copyright (C) 2004 Michael Niedermayer <michaelni at gmx.at>
> > + * Copyright (C) 2006 Robert Edele <yartrebo at earthlink.net>
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
> > +#include "obme.h"
> > +#include "h263.h"
> > +
> > +int ff_obmc_encode_init(OBMCContext *s, AVCodecContext *avctx)
> > +{
> > + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->
> pix_fmt);
> > + int plane_index, i, ret;
> > +
> > + #if FF_API_MOTION_EST
> > + FF_DISABLE_DEPRECATION_WARNINGS
> > + if (avctx->me_method == ME_ITER)
> > + s->motion_est = FF_ME_ITER;
> > + FF_ENABLE_DEPRECATION_WARNINGS
> > + #endif
> > +
> > + s->mv_scale = (avctx->flags & AV_CODEC_FLAG_QPEL) ? 2 : 4;
> > + s->block_max_depth= (avctx->flags & AV_CODEC_FLAG_4MV ) ? 1 : 0;
> > +
> > + avcodec_get_chroma_sub_sample(avctx->pix_fmt, &s->chroma_h_shift,
> &s->chroma_v_shift);
> > +
> > + for(plane_index=0; plane_index<3; plane_index++){
> > + s->plane[plane_index].diag_mc= 1;
> > + s->plane[plane_index].htaps= 6;
> > + s->plane[plane_index].hcoeff[0]= 40;
> > + s->plane[plane_index].hcoeff[1]= -10;
> > + s->plane[plane_index].hcoeff[2]= 2;
> > + s->plane[plane_index].fast_mc= 1;
> > + }
> > +
> > + ff_mpegvideoencdsp_init(&s->mpvencdsp, avctx);
> > +
>
> > + ff_obmc_alloc_blocks(s);
>
> missing return code check
>
>
> [...]
>
> > @@ -422,6 +345,7 @@ static int decode_blocks(SnowContext *s){
> > return res;
> > }
> > }
> > +
> > return 0;
> > }
> >
>
> stray change
>
>
> [...]
> > @@ -1582,124 +722,54 @@ static int encode_frame(AVCodecContext *avctx,
> AVPacket *pkt,
> > int hshift= i ? s->chroma_h_shift : 0;
> > int vshift= i ? s->chroma_v_shift : 0;
> > for(y=0; y<AV_CEIL_RSHIFT(height, vshift); y++)
> > - memcpy(&s->input_picture->data[i][y *
> s->input_picture->linesize[i]],
> > + memcpy(&s->obmc.input_picture->data[i][y *
> s->obmc.input_picture->linesize[i]],
> > &pict->data[i][y * pict->linesize[i]],
> > AV_CEIL_RSHIFT(width, hshift));
> > - s->mpvencdsp.draw_edges(s->input_picture->data[i],
> s->input_picture->linesize[i],
> > + s->obmc.mpvencdsp.draw_edges(s->obmc.input_picture->data[i],
> s->obmc.input_picture->linesize[i],
> > AV_CEIL_RSHIFT(width, hshift),
> AV_CEIL_RSHIFT(height, vshift),
> > EDGE_WIDTH >> hshift, EDGE_WIDTH >>
> vshift,
> > EDGE_TOP | EDGE_BOTTOM);
> >
> > }
> > emms_c();
> > - pic = s->input_picture;
> > + pic = s->obmc.input_picture;
> > pic->pict_type = pict->pict_type;
> > pic->quality = pict->quality;
> >
> > - s->m.picture_number= avctx->frame_number;
> > + s->obmc.m.picture_number= avctx->frame_number;
> > if(avctx->flags&AV_CODEC_FLAG_PASS2){
> > - s->m.pict_type = pic->pict_type = s->m.rc_context.entry[avctx->
> frame_number].new_pict_type;
> > + s->obmc.m.pict_type = pic->pict_type =
> s->obmc.m.rc_context.entry[avctx->frame_number].new_pict_type;
> > s->keyframe = pic->pict_type == AV_PICTURE_TYPE_I;
> > + s->obmc.keyframe = s->keyframe;
> > if(!(avctx->flags&AV_CODEC_FLAG_QSCALE)) {
> > - pic->quality = ff_rate_estimate_qscale(&s->m, 0);
> > + pic->quality = ff_rate_estimate_qscale(&s->obmc.m, 0);
> > if (pic->quality < 0)
> > return -1;
> > }
> > }else{
> > s->keyframe= avctx->gop_size==0 || avctx->frame_number %
> avctx->gop_size == 0;
> > - s->m.pict_type = pic->pict_type = s->keyframe ?
> AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
> > + s->obmc.m.pict_type = pic->pict_type = s->keyframe ?
> AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
> > + s->obmc.keyframe = s->keyframe;
> > }
> >
> > if(s->pass1_rc && avctx->frame_number == 0)
> > pic->quality = 2*FF_QP2LAMBDA;
> > if (pic->quality) {
> > s->qlog = qscale2qlog(pic->quality);
> > - s->lambda = pic->quality * 3/2;
> > + s->obmc.lambda = pic->quality * 3/2;
> > }
> > if (s->qlog < 0 || (!pic->quality && (avctx->flags &
> AV_CODEC_FLAG_QSCALE))) {
> > s->qlog= LOSSLESS_QLOG;
> > - s->lambda = 0;
> > + s->obmc.lambda = 0;
> > }//else keep previous frame's qlog until after motion estimation
> >
> > - if (s->current_picture->data[0]
> > -#if FF_API_EMU_EDGE
> > - && !(s->avctx->flags&CODEC_FLAG_EMU_EDGE)
> > -#endif
> > - ) {
> > - int w = s->avctx->width;
> > - int h = s->avctx->height;
> > -
> > - s->mpvencdsp.draw_edges(s->current_picture->data[0],
> > - s->current_picture->linesize[0], w ,
> h ,
> > - EDGE_WIDTH , EDGE_WIDTH , EDGE_TOP |
> EDGE_BOTTOM);
> > - if (s->current_picture->data[2]) {
> > - s->mpvencdsp.draw_edges(s->current_picture->data[1],
> > - s->current_picture->linesize[1],
> w>>s->chroma_h_shift, h>>s->chroma_v_shift,
> > - EDGE_WIDTH>>s->chroma_h_shift,
> EDGE_WIDTH>>s->chroma_v_shift, EDGE_TOP | EDGE_BOTTOM);
> > - s->mpvencdsp.draw_edges(s->current_picture->data[2],
> > - s->current_picture->linesize[2],
> w>>s->chroma_h_shift, h>>s->chroma_v_shift,
> > - EDGE_WIDTH>>s->chroma_h_shift,
> EDGE_WIDTH>>s->chroma_v_shift, EDGE_TOP | EDGE_BOTTOM);
> > - }
> > - }
> > -
> > - ff_snow_frame_start(s);
> > - av_frame_unref(avctx->coded_frame);
> > - ret = av_frame_ref(avctx->coded_frame, s->current_picture);
> > - if (ret < 0)
> > - return ret;
> > -
> > - s->m.current_picture_ptr= &s->m.current_picture;
> > - s->m.current_picture.f = s->current_picture;
> > - s->m.current_picture.f->pts = pict->pts;
> > - if(pic->pict_type == AV_PICTURE_TYPE_P){
> > - int block_width = (width +15)>>4;
> > - int block_height= (height+15)>>4;
> > - int stride= s->current_picture->linesize[0];
> > -
> > - av_assert0(s->current_picture->data[0]);
> > - av_assert0(s->last_picture[0]->data[0]);
> > -
> > - s->m.avctx= s->avctx;
> > - s->m. last_picture.f = s->last_picture[0];
> > - s->m. new_picture.f = s->input_picture;
> > - s->m. last_picture_ptr= &s->m. last_picture;
> > - s->m.linesize = stride;
> > - s->m.uvlinesize= s->current_picture->linesize[1];
> > - s->m.width = width;
> > - s->m.height= height;
> > - s->m.mb_width = block_width;
> > - s->m.mb_height= block_height;
> > - s->m.mb_stride= s->m.mb_width+1;
> > - s->m.b8_stride= 2*s->m.mb_width+1;
> > - s->m.f_code=1;
> > - s->m.pict_type = pic->pict_type;
> > -#if FF_API_MOTION_EST
> > - s->m.me_method= s->avctx->me_method;
> > -#endif
> > - s->m.motion_est= s->motion_est;
> > - s->m.me.scene_change_score=0;
> > - s->m.me.dia_size = avctx->dia_size;
> > - s->m.quarter_sample= (s->avctx->flags & AV_CODEC_FLAG_QPEL)!=0;
> > - s->m.out_format= FMT_H263;
> > - s->m.unrestricted_mv= 1;
> > -
> > - s->m.lambda = s->lambda;
> > - s->m.qscale= (s->m.lambda*139 + FF_LAMBDA_SCALE*64) >>
> (FF_LAMBDA_SHIFT + 7);
> > - s->lambda2= s->m.lambda2= (s->m.lambda*s->m.lambda +
> FF_LAMBDA_SCALE/2) >> FF_LAMBDA_SHIFT;
> > -
> > - s->m.mecc= s->mecc; //move
> > - s->m.qdsp= s->qdsp; //move
> > - s->m.hdsp = s->hdsp;
> > - ff_init_me(&s->m);
> > - s->hdsp = s->m.hdsp;
> > - s->mecc= s->m.mecc;
> > - }
> > -
> > if(s->pass1_rc){
> > memcpy(rc_header_bak, s->header_state, sizeof(s->header_state));
> > memcpy(rc_block_bak, s->block_state, sizeof(s->block_state));
> > }
> >
> > + ff_obmc_pre_encode_frame(&s->obmc, avctx, pict);
>
> missing return code check
>
>
> > +static void put_encoder_symbol(ObmcCoderContext *c, int ctx, int v,
> int sign)
> > +{
> > + SnowContext *s = (SnowContext *)c->avctx->priv_data;
> > + RangeCoder *rc = &s->c;
> > + uint8_t *state = s->block_state;
> > + if (c->priv_data) {
> > + RangeEncoderContext *coder = (RangeEncoderContext
> *)c->priv_data;
> > + rc = &coder->c; state = coder->state;
> > + }
> > + put_symbol(rc, &state[ctx], v, sign);
> > +}
>
> if theres a callback to encode a symbol, that should take a context,
> a symbol and a place to encode it to maybe.
>
> its very confusing and not well split if it checks private contexts
> and uses this to implement around the calling code using it to write
> to 2 different places
>
>
> > --- a/libavcodec/snow.c
> > +++ b/libavcodec/obmemc.c
> > @@ -1,5 +1,6 @@
> > /*
> > * Copyright (C) 2004 Michael Niedermayer <michaelni at gmx.at>
> > + * Copyright (C) 2006 Robert Edele <yartrebo at earthlink.net>
> > *
> > * This file is part of FFmpeg.
> > *
>
> What code from robert was added in obmemc.c compared to snow.c ?
> i see mostly code removials
>
>
> > @@ -628,7 +543,11 @@ static int halfpel_interpol(SnowContext *s, uint8_t
> *halfpel[4][4], AVFrame *fra
> > for(x=0; x<w; x++){
> > int i= y*ls + x;
> >
> > - halfpel[3][p][i]= (20*(src[i] + src[i+ls]) -
> 5*(src[i-ls] + src[i+2*ls]) + (src[i-2*ls] + src[i+3*ls]) + 16 )>>5;
> > + halfpel[3][p][i]= (
> > + 20*(src[i] + src[i+ls<h*w?i+ls:i-ls]) -
> > + 5*(src[i-ls<0?i+ls:i-ls] +
> src[i+2*ls<h*w?i+2*ls:i-2*ls]) +
> > + (src[i-2*ls<0?i+2*ls:i-2*ls] +
> src[i+3*ls<h*w?i+3*ls:i-3*ls]) + 16
> > + )>>5;
> > }
> > }
>
> i mentioned this previously
> the checks should be removed from the inner loop
> either by having seperate loops or allocating large enough arrays
>
>
>
> > @@ -511,22 +456,21 @@ fail:
> > return AVERROR(ENOMEM);
> > }
> >
> > -int ff_snow_common_init_after_header(AVCodecContext *avctx) {
> > - SnowContext *s = avctx->priv_data;
> > - int plane_index, level, orientation;
> > +int ff_obmc_common_init_after_header(OBMCContext *s)
> > +{
>
> > + int plane_index;//, level, orientation;
>
> stray comment
>
>
> > - s->max_ref_frames=1; //just make sure it's not an invalid value in
> case of no initial keyframe
> > - s->spatial_decomposition_count = 1;
> > + int width, height, i, j;
> > +
> > + width = avctx->width;
> > + height = avctx->height;
> >
> > ff_me_cmp_init(&s->mecc, avctx);
> > ff_hpeldsp_init(&s->hdsp, avctx->flags);
> > @@ -442,6 +394,8 @@ av_cold int ff_snow_common_init(AVCodecContext
> *avctx){
> > ff_dwt_init(&s->dwt);
> > ff_h264qpel_init(&s->h264qpel, 8);
> >
> > + s->max_ref_frames = 1
>
> lost comment
>
>
> > +int ff_obmc_encode_init(OBMCContext *s, AVCodecContext *avctx);
> > +int ff_obmc_pre_encode_frame(OBMCContext *f, AVCodecContext *avctx,
> const AVFrame *pict);
> > +void ff_obmc_encode_blocks(OBMCContext *s, int search);
>
> Missing API doxy-documentation
> How could anyone know how to use the factored out code with no docs
>
> [...]
>
>
> > --- a/HEAD^:libavcodec/snowenc.c
> > +++ b/HEAD:libavcodec/obme.c
> > @@ -1,5 +1,6 @@
> > /*
> > * Copyright (C) 2004 Michael Niedermayer <michaelni at gmx.at>
> > + * Copyright (C) 2006 Robert Edele <yartrebo at earthlink.net>
> > *
> > * This file is part of FFmpeg.
> > *
>
> git diff -w --stat HEAD^:libavcodec/snowenc.c HEAD:libavcodec/obme.c
> HEAD^:libavcodec/snowenc.c => HEAD:libavcodec/obme.c | 1114
> ++++++++++++++++++++++++++++++++----------------------------
> ------------------------------------------------------------
> ------------------------------------------------------------
> -------------------------------------------------------------------
> 1 file changed, 144 insertions(+), 970 deletions(-)
>
> The change mostly removes things, Where does the extra copyright come
> from?
> I would understand if you added yourself, as you work on the code
> but code from 2006 ?
> where is that code ?
>
>
> > - int plane_index, ret;
> > - int i;
> > + int plane_index, i, ret;
>
> stray change
>
>
> > @@ -214,7 +151,6 @@ static inline int get_penalty_factor(int lambda, int
> lambda2, int type){
> > }
> > }
> >
> > -//FIXME copy&paste
> > #define P_LEFT P[1]
> > #define P_TOP P[2]
> > #define P_TOPRIGHT P[3]
>
> Whatever the fixme meant, it should stay
>
>
> > @@ -266,7 +198,7 @@ static int encode_q_branch(SnowContext *s, int
> level, int x, int y){
> > int s_context= 2*left->level + 2*top->level + tl->level + tr->level;
> > int ref, best_ref, ref_score, ref_mx, ref_my;
> >
> > - av_assert0(sizeof(s->block_state) >= 256);
> > + //av_assert0(sizeof(s->block_state) >= 256);
> > if(s->keyframe){
> > set_blocks(s, level, x, y, pl, pcb, pcr, 0, 0, 0, BLOCK_INTRA);
> > return 0;
>
> Why is the assert disabled ?
>
>
> > - int stride= s->current_picture->linesize[0];
> > -
> > - av_assert0(s->current_picture->data[0]);
> > - av_assert0(s->last_picture[0]->data[0]);
> > -
> > - s->m.avctx= s->avctx;
> > - s->m. last_picture.f = s->last_picture[0];
> > - s->m. new_picture.f = s->input_picture;
> > - s->m. last_picture_ptr= &s->m. last_picture;
> > - s->m.linesize = stride;
> > - s->m.uvlinesize= s->current_picture->linesize[1];
> > - s->m.width = width;
> > - s->m.height= height;
> > - s->m.mb_width = block_width;
> > - s->m.mb_height= block_height;
> > - s->m.mb_stride= s->m.mb_width+1;
> > - s->m.b8_stride= 2*s->m.mb_width+1;
> > - s->m.f_code=1;
> > - s->m.pict_type = pic->pict_type;
> > + int stride= f->current_picture->linesize[0];
> > +
> > + av_assert0(f->current_picture->data[0]);
> > + av_assert0(f->last_pictures[0]->data[0]);
> > +
> > + f->m.avctx= f->avctx;
> > + f->m.last_picture.f = f->last_pictures[0];
> > + f->m.new_picture.f = f->input_picture;
> > + f->m.last_picture_ptr= &f->m.last_picture;
> > + f->m.linesize = stride;
> > + f->m.uvlinesize= f->current_picture->linesize[1];
> > + f->m.width = width;
> > + f->m.height= height;
> > + f->m.mb_width = block_width;
> > + f->m.mb_height= block_height;
> > + f->m.mb_stride= f->m.mb_width+1;
> > + f->m.b8_stride= 2*f->m.mb_width+1;
> > + f->m.f_code=1;
> > + /* s->m.pict_type = pic->pict_type; // DELETED */
> > #if FF_API_MOTION_EST
>
> Please dont rename variables, it makes reviewing the changes much
> harder
>
> Thanks
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The real ebay dictionary, page 2
> "100% positive feedback" - "All either got their money back or didnt
> complain"
> "Best seller ever, very honest" - "Seller refunded buyer after failed scam"
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
--
Станислав Долганов
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-factoring-obmc-out-of-snow.patch
Type: application/octet-stream
Size: 240731 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160829/a93fda13/attachment.obj>
More information about the ffmpeg-devel
mailing list