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

[pross at xvid.org]

On Thu, Jul 17, 2008 at 11:52:35PM +0200, Michael Niedermayer wrote:
> 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...)
> > > 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?

Solved. I had neglected the 12 byte padding enabled within lzo/av_memcpy_backptr,
and this was causing heap corruption upon av_free() cleanup.

> > -    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?

Affirm. Fixed.

> [...]
> > +        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.

Fixed.
-+            if (dst-offset<dst_start)
++            if (dst<dst_start+offset)

> [...]
> > +    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.

Roger that.

Updated decoder and avutil diff enclosed.
(Note to self: sync changelog and dox)

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-av_memcpy_backptr-r2.diff
Type: text/x-diff
Size: 2616 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080719/b1d6a9fb/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-eatgv-decode-r4.diff
Type: text/x-diff
Size: 12359 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080719/b1d6a9fb/attachment-0001.diff>
-------------- 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/20080719/b1d6a9fb/attachment.pgp>