[MPlayer-dev-eng] [PATCH] AVI LISTs should include pad bytes
Chris Pearson
ChrisCPearson at dsl-only.net
Thu Aug 19 02:49:21 CEST 2010
Hi,
I'd like to submit this patch to fix an issue in the AVI file encoding
produced by mencoder. This is my first patch for mplayer, so please
let me know if I'm not going about this the right way.
The problem I'm seeing is that, if a LIST chunk contains an odd-length
chunk, the LIST length does not include the pad byte following the
contained chunk.
That causes some AVI parsers to truncate the last chunk in the LIST.
For instance, ffmpeg, GStreamer avidemux, and the RIFFPad tool all
complain about this.
The attached RIFFPad snapshots show an example. In before.avi, the
video 'strf' chunk has an odd length (45 bytes). We know that it is
correctly followed by a pad byte because the offset of the next
chunk is even. However, the length of the containing 'strl' list is
odd and does not include the pad byte (4+8+56+8+45=121).
Because of that, the length of the next outer 'hrdl' list is also one short.
That causes truncation of the last chunk in the 'hdrl' list. That's
the audio 'strf' chunk in this case. Truncating it can break audio
playback because some decoders, e.g. Fluendo, require the WAVEFORMATEX
struct to be followed by the exactly 'cbSize' extra bytes (see 2nd
snapshot).
In after.avi, encoded with the patch, the lengths of the two containing
lists have been corrected. All other lengths and offsets are unchanged.
The patch includes a few other small changes that aren't critical:
- removed memset() to zeros following calloc(), since calloc()
does that
- replaced malloc() and memset() with calloc(), for consistency
- ensured that pad byte is always zero, even following a JUNK
chunk, per EA IFF 85 (pedantic, I agree :-)
- a couple minor optimizations
Again, please let me know if I'm missing anything. I may also try to
patch the GStreamer avidemux plug-in to work-around this issue.
Regards,
Chris Pearson
-------------- next part --------------
diff -Naur mplayer/libmpdemux/muxer_avi.c mplayer-avimux/libmpdemux/muxer_avi.c
--- mplayer/libmpdemux/muxer_avi.c 2010-08-18 12:23:54.000000000 -0700
+++ mplayer-avimux/libmpdemux/muxer_avi.c 2010-08-18 17:06:58.000000000 -0700
@@ -75,6 +75,12 @@
struct avi_odmlsuperidx_entry *superidx;
};
+// Returns length of padded chunk
+static inline int padded(int len)
+{
+ return len + (len & 1);
+}
+
static unsigned int avi_aspect(muxer_stream_t *vstream)
{
int x,y;
@@ -112,8 +118,7 @@
mp_msg(MSGT_MUXER, MSGL_ERR, "Too many streams! increase MUXER_MAX_STREAMS !\n");
return NULL;
}
- s=malloc(sizeof(muxer_stream_t));
- memset(s,0,sizeof(muxer_stream_t));
+ s=calloc(1, sizeof(*s));
if(!s) return NULL; // no mem!?
muxer->streams[muxer->avih.dwStreams]=s;
s->type=type;
@@ -121,13 +126,11 @@
s->timer=0.0;
s->size=0;
s->muxer=muxer;
- s->priv=si=malloc(sizeof(struct avi_stream_info));
- memset(si,0,sizeof(struct avi_stream_info));
+ s->priv=si=calloc(1, sizeof(*si));
si->idxsize=256;
si->idx=calloc(si->idxsize, sizeof(struct avi_odmlidx_entry));
si->riffofssize=16;
- si->riffofs=calloc((si->riffofssize+1), sizeof(off_t));
- memset(si->riffofs, 0, sizeof(off_t)*si->riffofssize);
+ si->riffofs=calloc(si->riffofssize+1, sizeof(off_t));
switch(type){
case MUXER_TYPE_VIDEO:
@@ -148,30 +151,25 @@
}
static void write_avi_chunk(stream_t *stream,unsigned int id,int len,void* data){
- int le_len = le2me_32(len);
- int le_id = le2me_32(id);
- stream_write_buffer(stream, &le_id, 4);
- stream_write_buffer(stream, &le_len, 4);
+ uint32_t riff[2] = {le2me_32(id), le2me_32(len)};
+ stream_write_buffer(stream, riff, sizeof(riff));
if(len>0){
if(data){
// DATA
stream_write_buffer(stream, data, len);
- if(len&1){ // padding
- unsigned char zerobyte=0;
- stream_write_buffer(stream, &zerobyte, 1);
- }
} else {
// JUNK
- char *avi_junk_data="[= MPlayer junk data! =]";
- if(len&1) ++len; // padding
- while(len>0){
- int l=strlen(avi_junk_data);
- if(l>len) l=len;
- stream_write_buffer(stream, avi_junk_data, l);
- len-=l;
+ static unsigned char avi_junk_data[]="[= MPlayer junk data! =]";
+ const int j=sizeof(avi_junk_data)-1;
+ for(int l=len;l>0;l-=j){
+ stream_write_buffer(stream, avi_junk_data, l>j?j:l);
}
}
+ if(len&1){ // padding (should be zero, even on junk)
+ unsigned char zerobyte=0;
+ stream_write_buffer(stream, &zerobyte, 1);
+ }
}
}
@@ -265,16 +263,10 @@
}
static void write_avi_list(stream_t *stream,unsigned int id,int len){
- unsigned int list_id=FOURCC_LIST;
- int le_len;
- int le_id;
- len+=4; // list fix
- list_id = le2me_32(list_id);
- le_len = le2me_32(len);
- le_id = le2me_32(id);
- stream_write_buffer(stream, &list_id, 4);
- stream_write_buffer(stream, &le_len, 4);
- stream_write_buffer(stream, &le_id, 4);
+ // Note: list len can never be odd, due to nested padding
+ uint32_t riff[3] = {
+ le2me_32(FOURCC_LIST), le2me_32(padded(len+4)), le2me_32(id)};
+ stream_write_buffer(stream, riff, sizeof(riff));
}
#define WFSIZE(wf) (sizeof(WAVEFORMATEX)+(wf)->cbSize)
@@ -319,13 +311,14 @@
/* fixup RIFF lengths */
for (i=0; i<=vsi->riffofspos; i++) {
rifflen = vsi->riffofs[i+1] - vsi->riffofs[i] - 8;
- movilen = le2me_32(rifflen - 12);
+ movilen = rifflen;
rifflen = le2me_32(rifflen);
stream_seek(muxer->stream, vsi->riffofs[i]+4);
stream_write_buffer(muxer->stream,&rifflen,4);
/* fixup movi length */
if (i > 0) {
+ movilen = le2me_32(movilen-12);
stream_seek(muxer->stream, vsi->riffofs[i]+16);
stream_write_buffer(muxer->stream,&movilen,4);
}
@@ -369,20 +362,20 @@
struct avi_stream_info *si = s->priv;
hdrsize+=12; // LIST
- hdrsize+=sizeof(muxer->streams[i]->h)+8; // strh
+ hdrsize+=padded(sizeof(muxer->streams[i]->h)+8); // strh
switch(muxer->streams[i]->type){
case MUXER_TYPE_VIDEO:
- hdrsize+=muxer->streams[i]->bih->biSize+8; // strf
- if (aspect != 0) {
- hdrsize+=8+4*(9+8*1); // vprp
- }
+ hdrsize+=padded(muxer->streams[i]->bih->biSize+8); // strf
+ if (aspect != 0) {
+ hdrsize+=padded(8+4*(9+8*1)); // vprp
+ }
break;
case MUXER_TYPE_AUDIO:
- hdrsize+=WFSIZE(muxer->streams[i]->wf)+8; // strf
+ hdrsize+=padded(WFSIZE(muxer->streams[i]->wf)+8); // strf
break;
}
if (isodml && si && si->superidx && si->superidxsize) {
- hdrsize += 32 + 16*si->superidxsize; //indx
+ hdrsize+=padded(32 + 16*si->superidxsize); //indx
}
}
write_avi_list(muxer->stream,listtypeAVIHEADER,hdrsize);
@@ -398,13 +391,13 @@
unsigned int idxhdr[8];
int j,n;
- hdrsize=sizeof(s->h)+8; // strh
+ hdrsize=padded(sizeof(s->h)+8); // strh
if (si && si->superidx && si->superidxsize) {
- hdrsize += 32 + 16*si->superidxsize; //indx
+ hdrsize += padded(32 + 16*si->superidxsize); //indx
}
switch(s->type){
case MUXER_TYPE_VIDEO:
- hdrsize+=s->bih->biSize+8; // strf
+ hdrsize+=padded(s->bih->biSize+8); // strf
s->h.fccHandler = s->bih->biCompression;
s->h.rcFrame.right = s->bih->biWidth;
s->h.rcFrame.bottom = s->bih->biHeight;
@@ -422,11 +415,11 @@
vprp.FieldInfo[0].CompressedBMWidth = muxer->avih.dwWidth;
vprp.FieldInfo[0].ValidBMHeight = muxer->avih.dwHeight;
vprp.FieldInfo[0].ValidBMWidth = muxer->avih.dwWidth;
- hdrsize+=8+4*(9+8*1); // vprp
+ hdrsize+=padded(8+4*(9+8*1)); // vprp
}
break;
case MUXER_TYPE_AUDIO:
- hdrsize+=WFSIZE(s->wf)+8; // strf
+ hdrsize+=padded(WFSIZE(s->wf)+8); // strf
s->h.fccHandler = s->wf->wFormatTag;
break;
}
@@ -612,7 +605,6 @@
si->superidxsize = si->superidxpos;
si->superidx = calloc(si->superidxsize, sizeof(*si->superidx));
- memset(si->superidx, 0, sizeof(*si->superidx) * si->superidxsize);
idxpos = 0;
for (j=0; j<si->superidxpos; j++) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-avimux-bug-1.jpg
Type: image/jpeg
Size: 85226 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100818/1a62cbac/attachment-0002.jpg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-avimumx-bug-2.jpg
Type: image/jpeg
Size: 54815 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100818/1a62cbac/attachment-0003.jpg>
More information about the MPlayer-dev-eng
mailing list