[FFmpeg-devel] Request for review

[Michael Niedermayer]

On Tue, Oct 21, 2008 at 10:08:47AM -0700, Roman V. Shaposhnik wrote:
> I would like to apply the following two patches which 
> are supposed to restructure the code a little bit and
> make it easier to take care of the huge tables later
> this week.
> 
> Please comment.

the ENABLE_SMALL patch looks good


[...]
> @@ -384,10 +401,20 @@ static inline void dv_decode_video_segment(DVVideoContext *s,
>      assert((((int)mb_bit_buffer)&7)==0);
>      assert((((int)vs_bit_buffer)&7)==0);
>  
> +    j    = buf_offset/150;
> +    chan = j/s->sys->difseg_size;
> +    seg  = j%s->sys->difseg_size;
> +    slot = ((buf_offset%150)*15 - 90)/(5<<4);

I think this could be put in dv_anchor or referenced by it, 
% and / are very slow.


> +
> +    /* in 1080i50 and 720p50 some seq are unused */
> +    if ((DV_PROFILE_IS_1080i50(s->sys) && chan != 0 && seg == 11) ||
> +        (DV_PROFILE_IS_720p50(s->sys) && seg > 9))
> +        return;

Can these not just be ommited from dv_anchor ?


[...]
> @@ -982,48 +1006,14 @@ static inline void dv_encode_video_segment(DVVideoContext *s,
>  
>  static int dv_decode_mt(AVCodecContext *avctx, void* sl)
>  {
> -    DVVideoContext *s = avctx->priv_data;
> -    int slice = (size_t)sl;
> -
> -    /* which DIF channel is this? */
> -    int chan = slice / (s->sys->difseg_size * 27);
> -
> -    /* slice within the DIF channel */
> -    int chan_slice = slice % (s->sys->difseg_size * 27);
> -
> -    /* byte offset of this channel's data */
> -    int chan_offset = chan * s->sys->difseg_size * 150 * 80;
> -
> -    /* DIF sequence */
> -    int seq = chan_slice / 27;
> -
> -    /* in 1080i50 and 720p50 some seq are unused */
> -    if ((DV_PROFILE_IS_1080i50(s->sys) && chan != 0 && seq == 11) ||
> -        (DV_PROFILE_IS_720p50(s->sys) && seq > 9))
> -        return 0;
> -
> -    dv_decode_video_segment(s, &s->buf[(seq*6+(chan_slice/3)+chan_slice*5+7)*80 + chan_offset],
> -                            &s->sys->video_place[slice*5]);
> +    dv_decode_video_segment((DVVideoContext *)avctx->priv_data, (size_t)sl);
>      return 0;
>  }

after this change dv_decode_mt() could maybe be ommited and
dv_decode_video_segment() be used directly


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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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/20081021/939b1005/attachment.pgp>