[FFmpeg-devel] [PATCH] Electronic Arts TGV decoder

[Michael Niedermayer]

On Wed, Jul 16, 2008 at 06:51:15PM +1000, pross at xvid.org wrote:
> On Mon, Jul 14, 2008 at 01:27:55PM +0200, Michael Niedermayer wrote:
> > On Mon, Jul 14, 2008 at 09:21:38PM +1000, pross at xvid.org wrote:
> > > On Sun, Jul 13, 2008 at 12:12:32AM +0200, Michael Niedermayer wrote:
> > > > On Sat, Jul 12, 2008 at 04:32:20PM +1000, pross at xvid.org wrote:
> > > > > On Thu, Jul 10, 2008 at 01:00:32AM +0200, Michael Niedermayer wrote:
> > > > > > On Wed, Jul 09, 2008 at 09:18:13PM +1000, pross at xvid.org wrote:
> > > > > > > Hi!
> > > > > > > 
> > > > > > > Second video codec in the EA series.
> > > > > > > 
> > > > > > > Samples: http://samples.mplayerhq.hu/game-formats/ea-tgv/
> > > > > > > Write-up: http://wiki.multimedia.cx/index.php?title=Electronic_Arts_TGV
> > > > > > [...]
> > > > > 
> > > > > Revised patch enclosed.
> > > > [...]
> > > Certainly worth testing. How do I "cleanly" allocate an AVFrame with
> > > linesize tied to width? (grepping lavc for -v DR1 examples was not 
> > > productive...)
> 
> Round three patches enclosed.
> 
> On my 400MHz PPC, removing DR1 infavour of simpler code results in negligible speed
> difference.
> 
> > my_frame.data[0]= av_malloc(width*height)
> > linesize[0]= width
> 
> Erm I tried the above, together with data[1]=av_malloc for the a palette, but
> received a segfault. Obviously I am neglecting something. Clues welcome.

you arent calling get/release_buffer() ? (you should not)
Anyway where does it segfault?


> 
> -- Peter
> (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

> Index: libavutil/lzo.c
> ===================================================================
> --- libavutil/lzo.c	(revision 14255)
> +++ libavutil/lzo.c	(working copy)
> @@ -107,24 +107,14 @@
>  }
>  
>  /**
> - * \brief copy previously decoded bytes to current position
> - * \param back how many bytes back we start
> + * \brief deliberate overlapping memcpy implementation
>   * \param cnt number of bytes to copy, must be >= 0
> + * \param back how many bytes back we start (the initial size of the overlapping window)
>   *
>   * cnt > back is valid, this will copy the bytes we just copied,
>   * thus creating a repeating pattern with a period length of back.
>   */
> -static inline void copy_backptr(LZOContext *c, int back, int cnt) {
> -    register const uint8_t *src = &c->out[-back];
> -    register uint8_t *dst = c->out;
> -    if (src < c->out_start || src > dst) {
> -        c->error |= LZO_INVALID_BACKPTR;
> -        return;
> -    }
> -    if (cnt > c->out_end - dst) {
> -        cnt = FFMAX(c->out_end - dst, 0);
> -        c->error |= LZO_OUTPUT_FULL;
> -    }
> +void av_memcpy_backptr(uint8_t *dst, const uint8_t *src, int cnt, int back) {

Arent back and src redundant relative to each other?


[...]

> Index: libavformat/electronicarts.c
> ===================================================================
> --- libavformat/electronicarts.c	(revision 14149)
> +++ libavformat/electronicarts.c	(working copy)

looks ok


[...]
> +        if (size1>0) {
> +            size -= size1;
> +            run = FFMIN(size1, dst_end-dst);
> +            memcpy(dst, src, run);
> +            dst += run;
> +            src += run;
> +        }
> +
> +        if (size2>0) {

> +            if (dst-offset<dst_start)

it should be dst-dst_start < offset because if dst is small nummerically
then dst-offset can overflow and result in a large pointer.


[...]
> +    if (avctx->get_buffer(avctx, &s->frame)<0) {
> +        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> +        return -1;
> +    }
> +    s->frame.linesize[0] = s->width;

That might work if you remove CODEC_CAP_DR1, but will break various players
if its not removed. The buffers provided by get_buffer with CODEC_CAP_DR1
could be in video memory or provided by a specific API like directdraw, SDL
or other and might not have a overrideable linesize.


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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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/20080717/50aedbb6/attachment.pgp>