[Ffmpeg-devel] [PATCH] mtv demuxer genesis

[Reynaldo H. Verdejo Pinochet]

On Wed, Oct 11, 2006 at 10:26:40PM +0200, Michael Niedermayer wrote:
> Hi
> > +    mtv->file_size = get_le32(pb);
> > +    mtv->segments = get_le32(pb);
> > +    url_fseek(pb, 32, SEEK_CUR);
> > +    mtv->audio_identifier = get_le24(pb);
> > +    mtv->audio_br = get_le16(pb);
> > +    mtv->img_colorfmt = get_le24(pb);
> > +    mtv->img_bpp = get_byte(pb);
> > +    mtv->img_width = get_le16(pb);
> > +    mtv->img_height = get_le16(pb);
> > +    mtv->img_segment_size = get_le16(pb);
> > +    url_fseek(pb, 4, SEEK_CUR);
> > +    mtv->audio_subsegments = get_le16(pb);
> 
> it might be a little picky (and no reason to reject the patch) but 
> why not format these like:
> mtv->file_size        = get_le32(pb);
> mtv->segments         = get_le32(pb);
> url_fskip(pb, 32);

Done.

> 
> why 33? this should be 64 i guess
> 

Fixed

> 
> [...]
> > +    if(url_feof(&s->pb))
> > +        return AVERROR_IO;
> 
> this isnt really needed, on EOF the following av_get_packet will fail and
> also cause AVERROR_IO to be returned but if you like ive no objections
> to leaving this here its just a little redundant
> 

Removed

> 
> > +
> > +    if((mtv->audio_subsegments / mtv->audio_packet_count) >= 1)
> 
> if(mtv->audio_subsegments >= mtv->audio_packet_count)
> 

duh .. :) fixed 

> 
> anyway, iam fine with the patch commit it after you fixed the above
> and reimar has no further objections

Thanks.

	Reynaldo
-------------- 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/20061011/37e0cb00/attachment.pgp>