[FFmpeg-devel] [PATCH] speex in ogg muxer

[Baptiste Coudurier]

Justin Ruggles wrote:
> Baptiste Coudurier wrote:
>> Justin Ruggles wrote:
>>> Baptiste Coudurier wrote:
>>>> Justin Ruggles wrote:
>>>>> Baptiste Coudurier wrote:
>>>>>> Justin Ruggles wrote:
>>>>>>> Baptiste Coudurier wrote:
>>>>>>>> Hi Justin,
>>>>>>>>
>>>>>>>> Justin Ruggles wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> This patch adds speex support to the ogg muxer.  It basically does the
>>>>>>>>> same thing as Ogg/FLAC, in that the 1st packet is a global header from
>>>>>>>>> extradata and the 2nd packet is vorbiscomment metadata.
>>>>>>>>>
>>>>>>>>> This seems to work just fine for speex-to-speex stream copy, but
>>>>>>>>> probably would not work for flv-to-speex because flv doesn't to have any
>>>>>>>>> speex extradata from what I can tell.  I guess a header could be
>>>>>>>>> constructed, but that would be a separate patch to the flv demuxer.
>>>>>>>>>
>>>>>>>>> This patch is a precursor to libspeex encoding support, which I'll be
>>>>>>>>> sending shortly.
>>>>>>>>>
>>>>>>>>> -Justin
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ------------------------------------------------------------------------
>>>>>>>>>
>>>>>>>>> Index: libavformat/oggenc.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- libavformat/oggenc.c	(revision 19244)
>>>>>>>>> +++ libavformat/oggenc.c	(working copy)
>>>>>>>>> @@ -104,17 +125,39 @@
>>>>>>>>>      bytestream_put_byte(&p, 0x00); // streaminfo
>>>>>>>>>      bytestream_put_be24(&p, 34);
>>>>>>>>>      bytestream_put_buffer(&p, streaminfo, FLAC_STREAMINFO_SIZE);
>>>>>>>>> -    oggstream->header_len[1] = 1+3+4+strlen(vendor)+4;
>>>>>>>>> -    oggstream->header[1] = av_mallocz(oggstream->header_len[1]);
>>>>>>>>> -    p = oggstream->header[1];
>>>>>>>>> +    p = ogg_write_vorbiscomment(4, bitexact, &oggstream->header_len[1]);
>>>>>>>>> +    if (!p)
>>>>>>>>> +        return -1;
>>>>>>>> AVERROR(ENOMEM)
>>>>>>> fixed.
>>>>>>>
>>>>>>>>> @@ -144,6 +188,12 @@
>>>>>>>>>                  av_log(s, AV_LOG_ERROR, "Extradata corrupted\n");
>>>>>>>>>                  av_freep(&st->priv_data);
>>>>>>>>>              }
>>>>>>>>> +        } else if (st->codec->codec_id == CODEC_ID_SPEEX) {
>>>>>>>>> +            if (ogg_build_speex_headers(st->codec, oggstream,
>>>>>>>>> +                                        st->codec->flags & CODEC_FLAG_BITEXACT) < 0) {
>>>>>>>>> +                av_log(s, AV_LOG_ERROR, "error writing Speex headers\n");
>>>>>>>>> +                av_freep(&st->priv_data);
>>>>>>>>> +            }
>>>>>>>> return error here with the return code of the func :>
>>>>>>>> Yes, it seems flac miss it too, this needs a fix.
>>>>>>>>
>>>>>>>> patch fine otherwise, maybe a micro bump for avformat would be nice.
>>>>>>> fixed. new patch attached. the new patch also differs in that it
>>>>>>> overrides the extra_headers field in the Speex header to be 0 since only
>>>>>>> the 2 required headers are written.
>>>>>>>
>>>>>> patch ok if it works :>
>>>>> Hmm... I've done several more tests and it does not quite work as-is for
>>>>> all samples.  Here is what I have run into.  The tests so far are for
>>>>> ogg-to-ogg stream copy.
>>>>>
>>>>> - When the source has more than 1 frame per packet, the resulting copy
>>>>> plays fine with ffmpeg/ffplay but is quick and choppy with speexdec.  I
>>>>> was able to fix this by modifying the ogg/speex demuxer to set
>>>>> avctx->frame_size to the number of samples in a packet instead of in a
>>>>> frame.  I also had to update the libspeex decoder accordingly.  Maybe
>>>>> this is the wrong way to go about it though.  I'm guessing it is a
>>>>> timestamp/granulepos issue, but I don't know enough about Ogg to tell
>>>>> more than that.
>>>> Sounds like a parser is needed.
>>> ah, indeed. that shouldn't be too difficult. i'll give it a try.
>>>
>>> once the frames are separated into individual AVPackets, what should i
>>> do in the ogg muxer if i want it to put multiple frames in a single ogg
>>> packet?
>> No you don't :)
> 
> That's not good.  Speex supports it for a reason.  With 1 frame per
> packet there is something like 30% container overhead.

Humm, well, I guess your problem is to set pkt->pts correctly in the
encoder and make sure pkt->duration is correctly set.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org