[FFmpeg-devel] [PATCH] Proper Interplay Video 16-bit support

[Kostya]

On Wed, Dec 23, 2009 at 11:40:26AM +0100, Diego Biurrun wrote:
> On Fri, Dec 18, 2009 at 10:56:48PM +0200, Kostya wrote:
> > Here's proper Interplay Video 16-bit support with separate decoder made
> > by svn cp from existing one. Demuxer now distinguishes them without any
> > problems too.
> > 
> > P.S. I'm pretty sure Diego will love this codec ID.
> 
> I still wonder why...

It's looooooooong and breaks vertical alignment somewhere.
 
> > P.D.F. I know it can be merged into single decoder but if you want that,
> > make it small or GSoC qualification task.
> 
> Oh, that will likely never happen, but you could accomplish it
> in hours.  Get that motivation up :)

Why do I have that strange feeling of deja vu?

> > --- libavcodec/ipvideo16.c	(revision 20496)
> > +++ libavcodec/ipvideo16.c	(working copy)
> > @@ -69,13 +69,22 @@
> >  
> > +#define CHECK_MV_PTR(n) \
> > +  if (s->mv_end - s->mv_ptr < n) { \
> > +    av_log(s->avctx, AV_LOG_ERROR, "Interplay video warning: mv_ptr out of bounds (%p >= %p)\n", \
> > +      s->mv_ptr + n, s->mv_end); \
> > +    return -1; \
> > +  }
> 
> Indentation is partly off and only two sapces.

Yep, totally inherited from original file. Why don't run through those
old files and reformat them a bit to modern standards. Nobody can beat
you in that area for sure.

Here's a quick list:
8bps.c
idcinvideo.c
smc.c
svq1dec.c
tscc.c

and everything with CHECK_STREAM_PTR

> > @@ -95,7 +104,7 @@
> >      }
> > -    s->dsp.put_pixels_tab[1][0](s->pixel_ptr, src->data[0] + motion_offset, s->stride, 8);
> > +    s->dsp.put_pixels_tab[0][0](s->pixel_ptr, src->data[0] + motion_offset, src->linesize[0], 8);
> 
> You could break that long line while you're at it.
> 
> > @@ -575,29 +571,28 @@
> >  
> > -    s->stride = s->current_frame.linesize[0];
> > -    s->stream_ptr = s->buf + 14;  /* data starts 14 bytes in */
> > -    s->stream_end = s->buf + s->size;
> > +    s->stride = s->current_frame.linesize[0] >> 1;
> > +    s->stream_ptr = s->buf + 16;  /* data starts 16 bytes in */
> > +    s->stream_end =
> > +    s->mv_ptr = s->buf + 14 + AV_RL16(s->buf+14);
> > +    s->mv_end = s->buf + s->size;
> >      s->line_inc = s->stride - 8;
> > -    s->upper_motion_limit_offset = (s->avctx->height - 8) * s->stride
> > -        + s->avctx->width - 8;
> > +    s->upper_motion_limit_offset = (s->avctx->height - 8) * s->current_frame.linesize[0]
> > +        + (s->avctx->width - 8) * 2;
> 
> Indentation is off.

The same mess as it was.
 
> >              debug_interplay("  block @ (%3d, %3d): encoding 0x%X, data ptr @ %p\n",
> > -                x - y, y / s->stride, opcode, s->stream_ptr);
> > +                x, y, opcode, s->stream_ptr);
> 
> ditto

ditto indeed

> > -            s->pixel_ptr = s->current_frame.data[0] + x;
> > +            s->pixel_ptr = s->current_frame.data[0] + x * 2 + y * s->current_frame.linesize[0];
> 
> another long line
> 
> >              ret = ipvideo_decode_block[opcode](s);
> >              if (ret != 0) {
> >                  av_log(s->avctx, AV_LOG_ERROR, " Interplay video: decode problem on frame %d, @ block (%d, %d)\n",
> > -                    frame, x - y, y / s->stride);
> > +                    frame, x, y);
> 
> ditto
> 
> Diego