[FFmpeg-devel] [PATCH] PB-frames support for i263

[Michael Niedermayer]

On Sat, Feb 21, 2009 at 10:34:05AM +0200, Kostya wrote:
> On Fri, Feb 20, 2009 at 06:52:12PM +0100, Michael Niedermayer wrote:
> > On Fri, Feb 20, 2009 at 08:17:24PM +0200, Kostya wrote:
> > > On Fri, Feb 20, 2009 at 01:09:08PM +0100, Michael Niedermayer wrote:
> > > > On Fri, Feb 20, 2009 at 11:37:40AM +0200, Kostya wrote:
> > > [...]
> > > > > 
> > > > > This one should be right.
> > > > 
> > > > ok
> > > 
> > > I'll apply it after code thaw then (i.e. next week).
> > 
> > you dont want to fix pb frames in the release?
> > well i dont care, as long as i dont have to redo the review ...
> 
> it's not fixing, it's rather new feature (i.e. not a thing for release) 

code that is under if(not true in current working cases) is not going to break
the release and thus there is
no sense to delay it also code that handles cases that just returned -1
before is also not going to break the release.
Compared to this many bug fixes which are needed have a very high potential
to break the release.

also iam considering to forbid any future freeze of head if development is
stoped like that. If the release manager considers a freeze needed that can
be done after forking and limited to the fork.
Its enough that development of core parts is stoped, stopping addition of
new features that cannot break existing feaures is just lame.

A freeze is NOT about only doing bug fixes (that may or may not break the
whole ffmpeg) BUT
its about NOT doing changes that could break the release unless they are
essential bug fixes.


[...]

> +static int h263_skip_b_part(MpegEncContext *s, int cbp)
> +{
> +    DECLARE_ALIGNED(16, DCTELEM, dblock[64]);
> +    int i, mbi = 0;
> +
> +    /* we have to set s->mb_intra to zero to decode B-part of PB-frame correctly
> +     * but real value should be restored in order to be used later (in OBMC condition)
> +     */
> +    FFSWAP(int, mbi, s->mb_intra);
> +    for (i = 0; i < 6; i++) {
> +        if (h263_decode_block(s, dblock, i, cbp&32) < 0)
> +            return -1;
> +        cbp+=cbp;
> +    }
> +    FFSWAP(int, mbi, s->mb_intra);

please dont use swaps they do 50% redundant operations.
this code is reasonable speed critial

[...]
> +    if(s->obmc && !s->mb_intra){
> +        if(s->pict_type == FF_P_TYPE && s->mb_x+1<s->mb_width && s->mb_num_left != 1)
> +            preview_obmc(s);
> +    }

ehh? !s->mb_intra && s->pict_type == FF_P_TYPE
redundant ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090221/64c53443/attachment.pgp>