[FFmpeg-devel] [PATCH] RealMedia muxer: support audio codecs other than AC-3

[Michael Niedermayer]

On Sun, May 02, 2010 at 10:36:22PM -0700, Baptiste Coudurier wrote:
> On 5/2/10 7:06 PM, Michael Niedermayer wrote:
>> On Sun, May 02, 2010 at 06:00:23PM -0400, Ronald S. Bultje wrote:
>>> Hi,
>>>
>>> On Sun, May 2, 2010 at 10:26 AM, Francesco Lavra
>>> <francescolavra at interfree.it>  wrote:
>>>> Updated patches attached.
>>> [..]
>>>> Index: libavformat/rm.h
>>>> ===================================================================
>>>> --- libavformat/rm.h	(revision 23008)
>>>> +++ libavformat/rm.h	(working copy)
>>>> @@ -23,9 +23,11 @@
>>>>   #define AVFORMAT_RM_H
>>>>
>>>>   #include "avformat.h"
>>>> +#include "riff.h"
>>>
>>> Why? Rest of patch #1 is OK.
>>>
>>> For patch #2:
>>>
>>>> Index: libavformat/rmenc.c
>>>> ===================================================================
>>>> --- libavformat/rmenc.c	(revision 23008)
>>>> +++ libavformat/rmenc.c	(working copy)
>>>> @@ -225,7 +225,15 @@
>>>>               put_be32(s, 0x10); /* unknown */
>>>>               put_be16(s, stream->enc->channels);
>>>>               put_str8(s, "Int0"); /* codec name */
>>>> +            if (!stream->enc->codec_tag)
>>>> +                stream->enc->codec_tag =
>>>> +                    ff_codec_get_tag(ff_rm_codec_tags, 
>>>> stream->enc->codec_id);
>>>> +            if (stream->enc->codec_tag) {
>>>> +                put_byte(s, 4); /* tag length */
>>>> +                put_le32(s, stream->enc->codec_tag);
>>>> +            } else {
>>>>               put_str8(s, "dnet"); /* codec name */
>>>> +            }
>>>
>>> The term spaghetti code comes to mind. This is of course not OK. I
>>> understand the original muxer was written for AC-3, but it's not OK to
>>> leave it as-is if you want to generalize that.
>>>
>>> I'd replace this whole thing. codec_tag is undefined for any practical
>>> purpose. Imagine one of the custom divx variants in AVI, it will set
>>> codec_tag. Do we want that in .rm? Probably not. If codec_id does not
>>> result in a good tag, then error out.
>>
>> if the user explitly aims somewhere and pulls the trigger
>> (that is set codec_tag without codec_id)
>> then we should do as he requests, it might even help stream copy of
>> unknown codecs
>
> Well this practice is IMHO ok for fairly generic containers such as AVI and 
> MOV. For others this is, again IMHO, looking for problems.

i think stream copy of codecs which we dont support should work.
for that though we must accept "unknown" codec tags without codec id
an actual example where someone wanted it and i fixed it was nellymoser
in flv before we had nelly support


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- 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/20100503/4767a34b/attachment.pgp>