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

[Francesco Lavra]

On Wed, 2010-05-19 at 19:31 +0200, Francesco Lavra wrote:
> 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.

Not anymore.

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

I realized now that the fix to libavformat/utils.c posted in this thread
could be useful for rmenc as well :)
Also, moved a cosmetic change to patch #3.
Updated patches attached.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 01_rm.patch
Type: text/x-patch
Size: 3503 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100522/f37e53e2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 02_rm.patch
Type: text/x-patch
Size: 2083 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100522/f37e53e2/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 03_rm.patch
Type: text/x-patch
Size: 1028 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100522/f37e53e2/attachment-0002.bin>