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

[Francesco Lavra]

And now back to the subject of this thread...

On Sun, 2010-05-02 at 18:00 -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.

Because AVCodecTag is defined there. In fact, compilation fails without
the #include.

> 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.

Based on the discussion in this thread, the attached implementation uses
codec_tag if codec_id is CODEC_ID_NONE, otherwise it looks for the tag
in ff_rm_codec_tags. In either case, if no valid value can be found for
codec_tag, the write_header function errors out.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 01_rm.patch
Type: text/x-patch
Size: 3499 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100519/fd4c1955/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 02_rm.patch
Type: text/x-patch
Size: 2208 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100519/fd4c1955/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 03_rm.patch
Type: text/x-patch
Size: 756 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100519/fd4c1955/attachment-0002.bin>