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

[Ronald S. Bultje]

Hi,

On Mon, May 3, 2010 at 7:34 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 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:
>>>> On Sun, May 2, 2010 at 10:26 AM, Francesco Lavra
>>>> <francescolavra at interfree.it> ?wrote:
>>>>> 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

I'm OK with doing it if codec_id is unset (CODEC_ID_NONE). In any
other case, codec_tag should be ignored (imo) and if codec_id is not
in our table, we should error out. This way, we force ourselves to add
the *correct* tag for each codec_id, rather than taking the AVI fourcc
and probably ending up creating files only mplayer can playback.

All imho of course.

Ronald