[FFmpeg-devel] [GSOC][PATCH 2/4] FFV1 p frames

[Moritz Barsnick]

On Sat, Aug 20, 2016 at 13:10:56 +0200, Michael Niedermayer wrote:
> On Thu, Aug 18, 2016 at 08:50:08PM +0200, Moritz Barsnick wrote:

> This code is based on libavcodec/snowdec.c:static int decode_q_branch
> Fixing formating is very welcome but it should be in a seperate patch

I thought it was being factored out, I didn't realize code copy was
still taking place. 

If even the formatting is copied, so is probably the majority of the
code? Can this duplication be avoided? Just wondering. Let me check...
See below.

> Or could you tell me what has changed between a reformatted
> decode_q_branch() and the original in libavcodec/snowdec.c
> thats not reformatted ?
> I cant easily, which would make review of this change much harder ...

True, though both are not identical, thanks to different choice of
variable names. But why is the code not shared? (I'm wondering, not
complaining - yet.)

As after applying patch 0001 and 0002, it looks like ffv1's
decode_q_branch() could be like this:
static int decode_q_branch(FFV1Context *f, int level, int x, int y) {
    // pseudo code, you get my jist:
    return snow::decode_q_branch(&f->ombc, level, x, y);
}

> I think this originate from:
> origin/master:libavcodec/snowenc.c:    pc.bytestream= p_buffer; //FIXME end/start? and at the other stoo
> origin/master:libavcodec/snowenc.c:    ic.bytestream= i_buffer; //FIXME end/start? and at the other stoo

I get the point.

> i think the FIXME should be kept, whatever it meant when code is
> factored either way this is not a fixme stanislav added so he cant
> know what it means ...

I agree, sorry for the noise.

Moritz