[FFmpeg-devel] [PATCH] ogg muxer

[Baptiste Coudurier]

M?ns Rullg?rd wrote:
> Baptiste Coudurier <baptiste.coudurier at smartjog.com> writes:
> 
> 
>>M?ns Rullg?rd wrote:
>>
>>>Baptiste Coudurier <baptiste.coudurier at smartjog.com> writes:
>>>
>>>
>>>
>>>>Hi
>>>>
>>>>$subject.
>>>
>>>
>>>Neat, but why?  Ogg is IMHO such a nasty format that its use should be
>>>actively discouraged.  Yet, there may be some reason to want this.
>>>
>>
>>Sake of completeness, I'd say.
>>
>>
>>>[...]
>>>
>>>page_segments = (size + 254) / 255;
>>
>>I prefer (size/255)+1 which illustrates better
>>what the code below does.
> 
> 
> Rethinking it, what I suggested is wrong for size%255 == 0.  What you
> want is page_segments = size/255 + !!size.

Well ok if you insist.

> [...] 
> 
>>>>+        if (st->codec->codec_id == CODEC_ID_VORBIS || st->codec->codec_id == CODEC_ID_THEORA) {
>>>
>>>
>>>Consider using a switch statement here to ease addition of more codecs.
>>
>>I'll change it when muxer supports more codecs.
> 
> 
> Why not now?  At the very least, split the line after the ||.
>

Ok.

> [...]
> 
> All else aside, you've forgotten to update allformats.c
> 

allformat.c already contains REGISTER_MUXDEMUX(OGG, ogg).

Another update.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ogg_muxer.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071029/5429c0cb/attachment.asc>