[FFmpeg-devel] [PATCH] MOV muxer : Add SoundDescriptionV2 support

[Jai Menon]

On Sat, Nov 28, 2009 at 02:00:39AM -0800, Baptiste Coudurier wrote:
> On 11/28/09 1:21 AM, Jai Menon wrote:
> >On Sat, Nov 28, 2009 at 12:03:47AM -0800, Baptiste Coudurier wrote:
> >>Hi,
> >>
> >>On 11/27/09 11:41 PM, Jai Menon wrote:
> >
> >[...]
> >
> >>>sounddescv2.patch
> >>>
> >>>
> >>>diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >>>index ac6378c..36b156e 100644
> >>>--- a/libavformat/movenc.c
> >>>+++ b/libavformat/movenc.c
> >>>@@ -405,13 +405,21 @@ static int mov_write_glbl_tag(ByteIOContext *pb, MOVTrack *track)
> >>>  static int mov_write_audio_tag(ByteIOContext *pb, MOVTrack *track)
> >>>  {
> >>>      int64_t pos = url_ftell(pb);
> >>>-    int version = track->mode == MODE_MOV&&
> >>>+    int version;
> >>>+
> >>>+    if (track->mode == MODE_MOV&&
> >>>          (track->audio_vbr ||
> >>>           track->enc->codec_id == CODEC_ID_PCM_S32LE ||
> >>>-         track->enc->codec_id == CODEC_ID_PCM_S24LE);
> >>>+         track->enc->codec_id == CODEC_ID_PCM_S24LE)) {
> >>>+        if (track->timescale>   UINT16_MAX)
> >>>+            version = 2;
> >>
> >>The stsd v2 must always be used when timescale>  UINT16_MAX
> >>regardless of codec.
> >
> >Yes, I agree, but right now I just dont have the time required to make
> >samples, remux and test against quicktime. Maybe someone who requires
> >this for other codecs can add it later?
> 
> Well, by experience, half-baked solutions for specific codecs rot
> forever in the codebase, so I would prefer avoiding it.

I don't think this is that big of a problem. The patch itself would be
trivial since it requires setting version outside of the current if
block -> ~2 line diff (unless I'm missing something). The issue as I
mentioned earlier is that I don't have a QT pro license so I dont
have the freedom to generate and test against a multitude of samples.
Also, there is the lack of time ;) I'm sure someone who cares about
other codecs will want to test it and maybe send a patch. But IMHO
that shouldn't get in the way of patches which improve the over all
situation.

> 
> >>>+        else version = 1;
> >>>+    }
> >>>
> >>>      put_be32(pb, 0); /* size */
> >>>-    put_le32(pb, track->tag); // store it byteswapped
> >>>+    if (version == 2)
> >>>+        put_le32(pb, AV_RL32("lpcm"));
> >>
> >>Technically all codecs can use stsd v2, I have an ALAC sample using
> >>stsd v2 since sample rate is 192000. It was just for test :)
> >
> >Okay, modified the patch to make it work for ALAC>  96Khz and others.
> >
> >>>+    else put_le32(pb, track->tag); // store it byteswapped
> >>>      put_be32(pb, 0); /* Reserved */
> >>>      put_be16(pb, 0); /* Reserved */
> >>>      put_be16(pb, 1); /* Data-reference index, XXX  == 1 */
> >>>@@ -444,6 +452,15 @@ static int mov_write_audio_tag(ByteIOContext *pb, MOVTrack *track)
> >>>          put_be32(pb, track->sampleSize / track->enc->channels); /* Bytes per packet */
> >>>          put_be32(pb, track->sampleSize); /* Bytes per frame */
> >>>          put_be32(pb, 2); /* Bytes per sample */
> >>>+    } else if (version == 2) {
> >>>+        put_be32(pb, 72);
> >>
> >>Please split it according to specs:
> >>
> >>    SInt16     always3;
> >>    SInt16     always16;
> >>    SInt16     alwaysMinus2;
> >>    SInt16     always0;
> >>    UInt32     always65536;
> >>    UInt32     sizeOfStructOnly;
> >
> >Sorry, I didnt understand this part, 72 is the
> >sizeof(SoundDescriptionV2), and the rest are subsequent values.
> >
> 
> Specs says stsd structure fields must be set to specific values,
> which is not done currently.

Ah, yes. Well recent Quicktime versions didnt really care so I didnt
bother either. But if it makes you happy, modified patch attached :)

-- 
Jai Menon

-------------- next part --------------
A non-text attachment was scrubbed...
Name: sounddescv2.patch
Type: text/x-patch
Size: 2325 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091128/77ed23e1/attachment.bin>