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

[Baptiste Coudurier]

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.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org