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

[Ronald S. Bultje]

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.

(The second chunk is OK.)

Ronald