[PATCH] Various updates to GIF demuxer
Hi, Attached patch adds support for several animated gif features that either weren't supported by or outright crashed mplayer, including variable frame sizes, interlacing, transparency, and some alternative frame update methods. Also fixed a bug extracting timestamps, which was probably introduced by a change in giflib, since the code in question dates back to rev 1.1. Samples (courtesy of gifanimations.com) here: ftp://opensource:opensource@ftp.on2.com/gifanimations_com-samples.tar.bz2 Comments/testing welcome. Cheers, John Index: libmpdemux/demux_gif.c =================================================================== RCS file: /cvsroot/mplayer/main/libmpdemux/demux_gif.c,v retrieving revision 1.5 diff -u -r1.5 demux_gif.c --- libmpdemux/demux_gif.c 5 Aug 2005 19:57:46 -0000 1.5 +++ libmpdemux/demux_gif.c 3 Feb 2006 20:50:45 -0000 @@ -19,8 +19,21 @@ #include "stheader.h" #include <gif_lib.h> -static int current_pts = 0; -static unsigned char *pallete = NULL; + +struct gif_priv_s { + GifFileType* gif_hndl; + unsigned char* pallete; + unsigned char* canvas; + unsigned int canvas_sz; +}; +#define gif_hndl (((struct gif_priv_s*)(demuxer->priv))->gif_hndl) +#define pallete (((struct gif_priv_s*)(demuxer->priv))->pallete) +#define canvas (((struct gif_priv_s*)(demuxer->priv))->canvas) +#define canvas_sz (((struct gif_priv_s*)(demuxer->priv))->canvas_sz) + +static int current_pts = 0, + ilace_offset[] = { 0, 4, 2, 1 }, + ilace_jmp[] = { 8, 8, 4, 2 }; #define GIF_SIGNATURE (('G' << 16) | ('I' << 8) | 'F') @@ -40,13 +53,16 @@ static int demux_gif_fill_buffer(demuxer_t *demuxer, demux_stream_t *ds) { - GifFileType *gif = (GifFileType *)demuxer->priv; + GifFileType *gif = gif_hndl; sh_video_t *sh_video = (sh_video_t *)demuxer->video->sh; GifRecordType type = UNDEFINED_RECORD_TYPE; int len = 0; demux_packet_t *dp = NULL; ColorMapObject *effective_map = NULL; char *buf = NULL; + int trans = 0; + int trans_color = 0; + int reaction = 0; while (type != IMAGE_DESC_RECORD_TYPE) { if (DGifGetRecordType(gif, &type) == GIF_ERROR) { @@ -70,8 +86,13 @@ } if (code == 0xF9) { int frametime = 0; - if (p[0] == 4) // is the length correct? - frametime = (p[1] << 8) | p[2]; // set the time, centiseconds + if (p[0] == 4) { // is the length correct? + frametime = (p[3] << 8) | p[2]; // set the time, centiseconds + reaction = (p[1] >> 2) & 0x3; + // Get transparancy info + trans = p[1] & 0x1; + trans_color = p[4]; + } current_pts += frametime; } else if ((code == 0xFE) && (verbose)) { // comment extension // print iff verbose @@ -103,21 +124,35 @@ } len = gif->Image.Width * gif->Image.Height; - dp = new_demux_packet(len); + dp = new_demux_packet(canvas_sz); buf = malloc(len); memset(buf, 0, len); - memset(dp->buffer, 0, len); + memcpy(dp->buffer, canvas, canvas_sz); - if (DGifGetLine(gif, buf, len) == GIF_ERROR) { - PrintGifError(); - return 0; // oops + if(gif->Image.Interlace) { + int i,j; + for(i = 0; i < 4; i++) { + for(j = ilace_offset[i]; j < gif->Image.Height; j += ilace_jmp[i]) { + if (DGifGetLine(gif, buf + (j * gif->Image.Width), gif->Image.Width) + == GIF_ERROR) + { + PrintGifError(); + return 0; // oops + } + } + } + } else { + if (DGifGetLine(gif, buf, len) == GIF_ERROR) { + PrintGifError(); + return 0; // oops + } } effective_map = gif->Image.ColorMap; if (effective_map == NULL) effective_map = gif->SColorMap; { - int y; + int y,x,width,height; // copy the pallete for (y = 0; y < 256; y++) { @@ -127,15 +162,55 @@ pallete[(y * 4) + 3] = 0; } - for (y = 0; y < gif->Image.Height; y++) { + // Use the canvas w/h as a bounding box for the blit + height = gif->Image.Height; + width = gif->Image.Width; + if(gif->Image.Top + gif->Image.Height > gif->SHeight) + height = gif->SHeight - gif->Image.Top; + if(gif->Image.Left + gif->Image.Width > gif->SWidth) + width = gif->SWidth - gif->Image.Left; + + for (y = 0; y < height; y++) { unsigned char *drow = dp->buffer; unsigned char *gbuf = buf + (y * gif->Image.Width); - drow += gif->Image.Width * (y + gif->Image.Top); + drow += gif->SWidth * (y + gif->Image.Top); drow += gif->Image.Left; - memcpy(drow, gbuf, gif->Image.Width); + if(!trans) { + // No transparancy, can do a fast blit + memcpy(drow, gbuf, width); + } else { + // Do a byte by byte blit for all non transparant pixels. + for (x = 0; x < width; x++) { + if(gbuf[x] != trans_color) + drow[x] = gbuf[x]; + } + } } + + switch(reaction) { + case 1: + // Preserve the current frame contents for the next frame. + memcpy(canvas, dp->buffer, canvas_sz); + break; + case 2: + // Fill the image area with the background color + for (y = 0; y < height; y++) { + unsigned char *drow = canvas; + drow += gif->SWidth * (y + gif->Image.Top); + drow += gif->Image.Left; + memset(drow, gif->SBackGroundColor, width); + } + break; + case 3: + // Preserve the previous frame contents for the next frame. + // No action required. + break; + default: + // Wipe the whole canvas with the background color + memset(canvas, gif->SBackGroundColor, canvas_sz); + } } free(buf); @@ -199,16 +274,19 @@ sh_video->bih->biCompression = sh_video->format; sh_video->bih->biBitCount = 8; sh_video->bih->biPlanes = 2; - pallete = (unsigned char *)(sh_video->bih + 1); + demuxer->priv = malloc(sizeof(struct gif_priv_s)); + pallete = (unsigned char *)(sh_video->bih + 1); + canvas_sz = gif->SWidth * gif->SHeight; + gif_hndl = gif; + canvas = malloc(canvas_sz); + memset(canvas, gif->SBackGroundColor, canvas_sz); - demuxer->priv = gif; - return demuxer; } static void demux_close_gif(demuxer_t* demuxer) { - GifFileType *gif = (GifFileType *)demuxer->priv; + GifFileType *gif = gif_hndl; if(!gif) return; @@ -217,6 +295,8 @@ PrintGifError(); demuxer->stream->fd = 0; + if(demuxer->priv) + free(demuxer->priv); demuxer->priv = NULL; }
On Fri, Feb 03, 2006 at 04:55:34PM -0500, John Koleszar wrote:
Attached patch adds support for several animated gif features that either weren't supported by or outright crashed mplayer, including variable frame sizes, interlacing, transparency, and some alternative frame update methods. Also fixed a bug extracting timestamps, which was probably introduced by a change in giflib, since the code in question dates back to rev 1.1.
Samples (courtesy of gifanimations.com) here: ftp://opensource:opensource@ftp.on2.com/gifanimations_com-samples.tar.bz2
Comments/testing welcome.
I'll test and commit if I find no problems. Thanks for the patch. --Joey -- "I can read it better than you can read it with your eyes closed." --Gu
On Fri, Feb 03, 2006 at 04:55:34PM -0500, John Koleszar wrote:
Attached patch adds support for several animated gif features that either weren't supported by or outright crashed mplayer, including variable frame sizes, interlacing, transparency, and some alternative frame update methods. Also fixed a bug extracting timestamps, which was probably introduced by a change in giflib, since the code in question dates back to rev 1.1.
Samples (courtesy of gifanimations.com) here: ftp://opensource:opensource@ftp.on2.com/gifanimations_com-samples.tar.bz2
Samples moved to http://samples.mplayerhq.hu/GIF/
--- libmpdemux/demux_gif.c 5 Aug 2005 19:57:46 -0000 1.5 +++ libmpdemux/demux_gif.c 3 Feb 2006 20:50:45 -0000 @@ -70,8 +86,13 @@ } if (code == 0xF9) { int frametime = 0; if (p[0] == 4) // is the length correct? - frametime = (p[1] << 8) | p[2]; // set the time, centiseconds + frametime = (p[3] << 8) | p[2]; // set the time, centiseconds
This is the frametime fix, compare http://www.onicos.com/staff/iz/formats/gif.html#gceb to see that if p[0] is block size p[3] and p[2], not p[1] and p[2] must be the delay time. This hunk committed, it fixes several of the samples above. Diego
On Sat, Jan 13, 2007 at 06:59:26AM +0100, Diego Biurrun wrote:
On Fri, Feb 03, 2006 at 04:55:34PM -0500, John Koleszar wrote:
Attached patch adds support for several animated gif features that either weren't supported by or outright crashed mplayer, including variable frame sizes, interlacing, transparency, and some alternative frame update methods. Also fixed a bug extracting timestamps, which was probably introduced by a change in giflib, since the code in question dates back to rev 1.1.
--- libmpdemux/demux_gif.c 5 Aug 2005 19:57:46 -0000 1.5 +++ libmpdemux/demux_gif.c 3 Feb 2006 20:50:45 -0000 @@ -70,8 +86,13 @@ } if (code == 0xF9) { int frametime = 0; if (p[0] == 4) // is the length correct? - frametime = (p[1] << 8) | p[2]; // set the time, centiseconds + frametime = (p[3] << 8) | p[2]; // set the time, centiseconds
This is the frametime fix, compare
http://www.onicos.com/staff/iz/formats/gif.html#gceb
to see that if p[0] is block size p[3] and p[2], not p[1] and p[2] must be the delay time.
This hunk committed, it fixes several of the samples above.
Here is the rest of the patch, updated for Subversion HEAD, plus a few cosmetical fixes. Diego
On Sat, Jan 13, 2007 at 07:15:22AM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 06:59:26AM +0100, Diego Biurrun wrote:
On Fri, Feb 03, 2006 at 04:55:34PM -0500, John Koleszar wrote:
Attached patch adds support for several animated gif features that either weren't supported by or outright crashed mplayer, including variable frame sizes, interlacing, transparency, and some alternative frame update methods. Also fixed a bug extracting timestamps, which was probably introduced by a change in giflib, since the code in question dates back to rev 1.1.
--- libmpdemux/demux_gif.c 5 Aug 2005 19:57:46 -0000 1.5 +++ libmpdemux/demux_gif.c 3 Feb 2006 20:50:45 -0000 @@ -70,8 +86,13 @@ } if (code == 0xF9) { int frametime = 0; if (p[0] == 4) // is the length correct? - frametime = (p[1] << 8) | p[2]; // set the time, centiseconds + frametime = (p[3] << 8) | p[2]; // set the time, centiseconds
This is the frametime fix, compare
http://www.onicos.com/staff/iz/formats/gif.html#gceb
to see that if p[0] is block size p[3] and p[2], not p[1] and p[2] must be the delay time.
This hunk committed, it fixes several of the samples above.
Here is the rest of the patch, updated for Subversion HEAD, plus a few cosmetical fixes.
I think it's time that I found a club for scatterbrained patch attachers. Reimar will be invited in as exclusive member... Diego
Hello, On Sat, Jan 13, 2007 at 10:01:49AM +0100, Diego Biurrun wrote:
+struct gif_priv_s { + GifFileType* gif_hndl; + unsigned char* palette; + unsigned char* canvas; + unsigned int canvas_sz; +}; +#define gif_hndl (((struct gif_priv_s*)(demuxer->priv))->gif_hndl) +#define palette (((struct gif_priv_s*)(demuxer->priv))->palette) +#define canvas (((struct gif_priv_s*)(demuxer->priv))->canvas) +#define canvas_sz (((struct gif_priv_s*)(demuxer->priv))->canvas_sz)
Very ugly IMO.
+static int current_pts = 0, + ilace_offset[] = { 0, 4, 2, 1 }, + ilace_jmp[] = { 8, 8, 4, 2 };
More evil global variables.
- // copy the palette + // Copy the palette.
Cosmetics and in general multiple unrelated issues mixed in one patch. [...]
+ // Do a byte by byte blit for all non-transparent pixels. + for (x = 0; x < width; x++) { + if(gbuf[x] != trans_color) + drow[x] = gbuf[x]; + } [...] + // Fill the image area with the background color. + for (y = 0; y < height; y++) { + unsigned char *drow = canvas; + drow += gif->SWidth * (y + gif->Image.Top); + drow += gif->Image.Left; + memset(drow, gif->SBackGroundColor, width); + }
I'm sure the performance of these could be optimized a lot. Don't know if that is important though.
static void demux_close_gif(demuxer_t* demuxer) { - GifFileType *gif = (GifFileType *)demuxer->priv; + GifFileType *gif = (demuxer->priv)?gif_hndl:NULL;
if(!gif) return;
I'm not sure that gif_hndl != NULL always is a valid assumption. If it is not, this code would leave demuxer->priv unfreed. Since this code needs an overhaul anyway, I'm for applying my patch first. Greetings, Reimar Döffinger
On Sat, Jan 13, 2007 at 10:32:08AM +0100, Reimar Döffinger wrote:
On Sat, Jan 13, 2007 at 10:01:49AM +0100, Diego Biurrun wrote:
- // copy the palette + // Copy the palette.
Cosmetics and in general multiple unrelated issues mixed in one patch.
This cosmetical change is my mistake, it slipped in while I ported the patch.
Since this code needs an overhaul anyway, I'm for applying my patch first.
If you are willing to clean it up and merge this patch I'll be delighted, but if you just want to commit some cleanup and not merge this patch I'd be unhappy. This adds support for many unsupported files that otherwise crash MPlayer ... Diego
Hello, On Sat, Jan 13, 2007 at 10:38:20AM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 10:32:08AM +0100, Reimar Döffinger wrote:
Since this code needs an overhaul anyway, I'm for applying my patch first.
If you are willing to clean it up and merge this patch I'll be delighted, but if you just want to commit some cleanup and not merge this patch I'd be unhappy. This adds support for many unsupported files that otherwise crash MPlayer ...
Since the code IMO is not okay for inclusion as it is I don't see much difference if my patch is applied or not. Also if there are crashes, esp. if we do not know where and why they should be fixed before adding loads of features. Greetings, Reimar Döffinger
On Sat, Jan 13, 2007 at 11:56:21AM +0100, Reimar Döffinger wrote:
On Sat, Jan 13, 2007 at 10:38:20AM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 10:32:08AM +0100, Reimar Döffinger wrote:
Since this code needs an overhaul anyway, I'm for applying my patch first.
If you are willing to clean it up and merge this patch I'll be delighted, but if you just want to commit some cleanup and not merge this patch I'd be unhappy. This adds support for many unsupported files that otherwise crash MPlayer ...
Since the code IMO is not okay for inclusion as it is I don't see much difference if my patch is applied or not. Also if there are crashes, esp. if we do not know where and why they should be fixed before adding loads of features.
Here are two files that crash: http://samples.mplayerhq.hu/GIF/2c-button_green_blue2.gif http://samples.mplayerhq.hu/GIF/2c_button-yellow_red.gif Diego
On Sat, Jan 13, 2007 at 12:41:13PM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 11:56:21AM +0100, Reimar Döffinger wrote:
On Sat, Jan 13, 2007 at 10:38:20AM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 10:32:08AM +0100, Reimar Döffinger wrote:
Since this code needs an overhaul anyway, I'm for applying my patch first.
If you are willing to clean it up and merge this patch I'll be delighted, but if you just want to commit some cleanup and not merge this patch I'd be unhappy. This adds support for many unsupported files that otherwise crash MPlayer ...
Since the code IMO is not okay for inclusion as it is I don't see much difference if my patch is applied or not. Also if there are crashes, esp. if we do not know where and why they should be fixed before adding loads of features.
Here are two files that crash:
http://samples.mplayerhq.hu/GIF/2c-button_green_blue2.gif http://samples.mplayerhq.hu/GIF/2c_button-yellow_red.gif
The crashes are gone, thanks a bundle. Here are some examples of files that currently play incorrectly but work fine with John's patch: http://samples.mplayerhq.hu/GIF/3d_book2.gif http://samples.mplayerhq.hu/GIF/3D.gif http://samples.mplayerhq.hu/GIF/7up.gif http://samples.mplayerhq.hu/GIF/artists.gif Diego
Hello, On Sat, Jan 13, 2007 at 01:06:27PM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 12:41:13PM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 11:56:21AM +0100, Reimar Döffinger wrote:
On Sat, Jan 13, 2007 at 10:38:20AM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 10:32:08AM +0100, Reimar Döffinger wrote:
Since this code needs an overhaul anyway, I'm for applying my patch first.
If you are willing to clean it up and merge this patch I'll be delighted, but if you just want to commit some cleanup and not merge this patch I'd be unhappy. This adds support for many unsupported files that otherwise crash MPlayer ...
Since the code IMO is not okay for inclusion as it is I don't see much difference if my patch is applied or not. Also if there are crashes, esp. if we do not know where and why they should be fixed before adding loads of features.
Here are two files that crash:
http://samples.mplayerhq.hu/GIF/2c-button_green_blue2.gif http://samples.mplayerhq.hu/GIF/2c_button-yellow_red.gif
The crashes are gone, thanks a bundle.
Here are some examples of files that currently play incorrectly but work fine with John's patch:
http://samples.mplayerhq.hu/GIF/3d_book2.gif http://samples.mplayerhq.hu/GIF/3D.gif http://samples.mplayerhq.hu/GIF/7up.gif http://samples.mplayerhq.hu/GIF/artists.gif
I just downloaded the whole directory before starting anyway. They all look okay to me, though I think there are no files that use what I called "refmode" 2. Also many seem to have transparency set without actually making use of it, what a waste... Greetings, Reimar Döffinger
On Sat, Jan 13, 2007 at 02:03:00PM +0100, Reimar Döffinger wrote:
On Sat, Jan 13, 2007 at 01:06:27PM +0100, Diego Biurrun wrote:
Here are some examples of files that currently play incorrectly but work fine with John's patch:
http://samples.mplayerhq.hu/GIF/3d_book2.gif http://samples.mplayerhq.hu/GIF/3D.gif http://samples.mplayerhq.hu/GIF/7up.gif http://samples.mplayerhq.hu/GIF/artists.gif
I just downloaded the whole directory before starting anyway. They all look okay to me, though I think there are no files that use what I called "refmode" 2. Also many seem to have transparency set without actually making use of it, what a waste...
These two are still not painted correctly, but work with John's patch: http://samples.mplayerhq.hu/GIF/7up.gif http://samples.mplayerhq.hu/GIF/houses_002.gif Just load them in a browser to see what they are supposed to look like. Diego
On Mon, Jan 15, 2007 at 06:32:16AM +0100, Diego Biurrun wrote:
On Sat, Jan 13, 2007 at 02:03:00PM +0100, Reimar Döffinger wrote:
On Sat, Jan 13, 2007 at 01:06:27PM +0100, Diego Biurrun wrote:
Here are some examples of files that currently play incorrectly but work fine with John's patch:
http://samples.mplayerhq.hu/GIF/3d_book2.gif http://samples.mplayerhq.hu/GIF/3D.gif http://samples.mplayerhq.hu/GIF/7up.gif http://samples.mplayerhq.hu/GIF/artists.gif
I just downloaded the whole directory before starting anyway. They all look okay to me, though I think there are no files that use what I called "refmode" 2. Also many seem to have transparency set without actually making use of it, what a waste...
These two are still not painted correctly, but work with John's patch:
http://samples.mplayerhq.hu/GIF/7up.gif http://samples.mplayerhq.hu/GIF/houses_002.gif
Just load them in a browser to see what they are supposed to look like.
Reimar implemented support for interlaced GIFs, so all samples work fine now and this patch is no longer needed. Thanks a bunch Reimar. Diego
participants (4)
-
Diego Biurrun -
Joey Parrish -
John Koleszar -
Reimar Döffinger