[FFmpeg-devel] [PATCH] SoX native format muxer and demuxer

[Michael Niedermayer]

On Thu, May 28, 2009 at 01:16:00PM -0400, Daniel Verkamp wrote:
> On Thu, May 28, 2009 at 9:31 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, May 27, 2009 at 9:44 PM, Daniel Verkamp <daniel at drv.nu> wrote:
> > [...]
> >
> >> + ? ?le = (get_le32(pb) == SOX_TAG);
> >> +
> >> + ? ?st = av_new_stream(s, 0);
> >> + ? ?if (!st)
> >> + ? ? ? ?return AVERROR(ENOMEM);
> >> +
> >> + ? ?st->codec->codec_type = CODEC_TYPE_AUDIO;
> >> +
> >> + ? ?if (le) {
> >
> > is the temporary le even needed ?
> >
> 
> No, removed.
> 
> >
> > [...]
> >> + ? ?if ((header_size + 4) & 7 || header_size < SOX_FIXED_HDR + comment_size
> >> + ? ? ? ?|| st->codec->channels > 65535) /* Reserve top 16 bits */ {
> >> + ? ? ? ?av_log(s, AV_LOG_ERROR, "invalid header\n");
> >> + ? ? ? ?return -1;
> >> + ? ?}
> >> +
> >> + ? ?if (comment_size) {
> >> + ? ? ? ?char *comment = av_mallocz(comment_size + FF_INPUT_BUFFER_PADDING_SIZE);
> >
> > that still could overflow with a suitable FF_INPUT_BUFFER_PADDING_SIZE
> 
> I assumed FF_INPUT_BUFFER_PADDING_SIZE wouldn't change; I guess that's
> not a good idea.
> 
> New check for overflow added and check for valid comment_size cleaned up.
> 
> > you know what the next line would do in that case?
> >
> 
> Yes, I see it now - comment_size will be large while the allocated
> area will be small, leading to buffer overflow.
> 
> >> + ? ? ? ?if (get_buffer(pb, comment, comment_size) != comment_size) {
> >
> > [...]
> >> + ? ?for (pad = comment_size - comment_len; pad; pad--)
> >> + ? ? ? ?put_byte(pb, 0);
> >
> > the pad variable is superflous
> >
> 
> Removed.
> 
> >
> > [...]
> >> +static int sox_write_trailer(AVFormatContext *s)
> >> +{
> >> + ? ?SoXContext *sox = s->priv_data;
> >> + ? ?ByteIOContext *pb = s->pb;
> >> + ? ?AVCodecContext *enc = s->streams[0]->codec;
> >
> >> + ? ?int64_t file_size;
> >> +
> >> + ? ?if (!url_is_streamed(s->pb)) {
> >> + ? ? ? ?int64_t num_samples;
> >> + ? ? ? ?/* update number of samples */
> >> + ? ? ? ?file_size = url_ftell(pb);
> >> + ? ? ? ?num_samples = (file_size - sox->header_size - 4) >> 2LL;
> >
> > declaration & init can be merged
> >
> 
> Done.
> 
> Thanks,
> -- Daniel Verkamp

> From 6189eb6c9169749187e1b84a2929e17582f64b3f Mon Sep 17 00:00:00 2001
> From: Daniel Verkamp <daniel at drv.nu>
> Date: Thu, 28 May 2009 12:59:01 -0400
> Subject: [PATCH] SoX native format muxer and demuxer

looks ok

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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- 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/20090528/b0285eed/attachment.pgp>