[FFmpeg-devel] [PATCH] Matroska Muxer

[David Conrad]

On Aug 30, 2007, at 4:30 AM, Diego Biurrun wrote:

> On Thu, Aug 30, 2007 at 12:10:24AM -0400, David Conrad wrote:
>>
>> New patch attached, with regression tests updated.
>
> Documentation and build system parts are OK, I'm not sure if I already
> said this.
>
> Some unimportant nits below.
>
>> --- /dev/null
>> +++ b/libavformat/matroskaenc.c
>> @@ -0,0 +1,828 @@
>> +
>> +/**
>> + * Write an EBML size meaning "unknown size"
>
> missing period

Fixed

>
>> + * @param segment_offset The absolute offset to the position in  
>> the file
>> + *                       where the segment begins
>
> missing period

Fixed

>
>> + * @param numelements the maximum number of elements that will be  
>> indexed
>
> Capitalize

Fixed

>
>> + * @return the file offset where the seekhead was written
>
> both

Fixed

>
>> +        av_log(s, AV_LOG_WARNING, "no AAC extradata, unable to  
>> determine samplerate\n");
>
> ditto

Fixed

>
>> +            av_log(s, AV_LOG_ERROR, "no bmp codec id found");
>
> Ditto, and I'd write "ID".

Fixed

>
>> +            av_log(s, AV_LOG_ERROR, "no wav codec id found");
>
> same here

Fixed

>
> There are some more instances of lowercase ID, I prefer uppercase.   
> Feel
> free to ignore me.

I changed the two lowercase IDs I in comments; were there others?

>
>
> Overall this is looking good.  I expect your muxer to become an  
> official
> part of FFmpeg soon.
>
> Diego
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel