[Ffmpeg-devel] Re: [PATCH] FFmpeg theora encoding

[Michael Niedermayer]

Hi

On Mon, Jan 08, 2007 at 09:20:19AM +0000, Paul Richards wrote:
> Here are the file rename, whitespace, and ordering changes you requested.
> 
> 
> On 07/01/07, Diego Biurrun <diego at biurrun.de> wrote:
> >On Sun, Jan 07, 2007 at 08:54:13PM +0000, Paul Richards wrote:
> >> On 07/01/07, Paul Richards <paul.richards at gmail.com> wrote:
> >> >After taking in the various comments given here is the revised patch.
> >> >Perhaps it can be considered for committing.
> >> >
> >> >Unfortunately the VLC playback problem remains but I cannot yet
> >> >identify the source of the problem.  Hopefully this can be resolved in
> >> >a followup commit.
> >>
> >> Here is another attempt.  This time I have refactored code which
> >> checks libtheora return values.
> >>
> >> Hopefully this one is good enough to commit? :p
> >
> >You still have trailing whitespace in the file and it has not been
> >renamed to libtheoraenc yet.  Also, please respect alphabetical order in
> >the configure script.
> >
> >Diego
> >_______________________________________________
> >ffmpeg-devel mailing list
> >ffmpeg-devel at mplayerhq.hu
> >http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
> >
> 
> 
> -- 
> Paul Richards

[makefile/configre stuff which iam not maintaining]

[...]
>  OBJS-$(CONFIG_THEORA_DECODER)          += vp3.o
> +OBJS-$(CONFIG_THEORA_ENCODER)          += libtheoraenc.o

this should be CONFIG_LIBTHEORA_ENCODER or something CONFIG_THEORA_ENCODER is
"reserved" for a possible future native encoder


[...]
> +/*!

as i said in some other mail /** and @abc is prefered over /*! \abd
because its more common in ffmpeg (minor issue, id accpet the patch with
the /*! \abc too)


[...]
> +static void concatenate_packet(unsigned int* offset, AVCodecContext* avc_context, const ogg_packet* packet)
> +{
> +    avc_context->extradata_size += 2 + packet->bytes;
> +    avc_context->extradata = av_realloc( avc_context->extradata, avc_context->extradata_size );
> +    avc_context->extradata[ (*offset)++ ] = packet->bytes >> 8;
> +    avc_context->extradata[ (*offset)++ ] = packet->bytes & 0xff;
> +    memcpy( avc_context->extradata + (*offset), packet->packet, packet->bytes );
> +    (*offset) += packet->bytes;

avc_context->extradata_size += 2 + packet->bytes;
could theoretically overflow (a huge comment packet provided by a mallicious
source file maybe)
after that the memcpy will cause a heap overflow ...

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

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070111/69160bb3/attachment.pgp>