[FFmpeg-devel] [PATCH] WebM mux/demux

[David Conrad]

On May 19, 2010, at 3:26 PM, James Zern wrote:

> On Wed, May 19, 2010 at 15:14, David Conrad <lessen42 at gmail.com> wrote:
>> On May 19, 2010, at 2:51 PM, James Zern wrote:
>> 
>>> Related thread: '[PATCH] VP8 de/encode via libvpx'
>>> VP8 and associated WebM container made public today [1].
>>> 
>>> The attached adds WebM de/muxing to matroska.
>>> For both some extra ifdef's/checks were added to only allow VP8/vorbis
>>> as that is the current supported configuration in web browsers. If
>>> these are too busy in this respect they be cleaned up.
>> 
>> Yes, the only #ifdef should be around the AVOutputFormat.
>> 
>>> +    if (!strcmp("webm", s->oformat->name)) {
>>> +        mkv->max_cluster_size = 500*1024*1024;   // 500 MB
>>> +        mkv->max_cluster_pts = 30000;            // 30 seconds
>>> +    } else {
>> 
>> Why? Seeking can only seek to a cluster, then do a linear scan to find the needed packet, and 500 MB makes that incredibly inefficient.
> This was added as a worst case, the recommendation is for keyframes on
> the 2-5s range.

>>> +static int mkv_copy_packet(MatroskaMuxContext *mkv, const AVPacket *pkt)
>>> +{
>>> +    uint8_t *data = mkv->cur_audio_pkt.data;
>>> +    mkv->cur_audio_pkt = *pkt;
>>> +    mkv->cur_audio_pkt.data = av_fast_realloc(data, &mkv->audio_buffer_size, pkt->size);
>>> +    if (mkv->cur_audio_pkt.data == NULL)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    memcpy(mkv->cur_audio_pkt.data, pkt->data, pkt->size);
>>> +    mkv->cur_audio_pkt.size = pkt->size;
>>> +    return 0;
>>> +}
>> 
>>> +
>>> +static int mkv_write_packet2(AVFormatContext *s, AVPacket *pkt)
>>> +{
>> 
>> webm and mkv shouldn't use separate write_packets, the interleaving changes should be done for mkv too. Maybe call the current mkv_write_packet mkv_write_packet_internal and move all cluster handling to the outer one.
>> 
> OK. The thought initially was not to force the keyframe change on the
> matroska muxer, but I think this can be reworked.

If the conceptual goal is 1 cluster == 1 gop, the main reason I didn't do it for mkv is because I was worried about overhead. But after making the the streaming changes I think the overhead of this shouldn't be terribly important. At any rate, the only difference I want in muxing webm is the doctype and checks to not write unsupported elements (namely non-vp8+vorbis.)

>>> +    MatroskaMuxContext *mkv = s->priv_data;
>>> +    ByteIOContext *pb = s->pb;
>>> +    AVCodecContext *codec = s->streams[pkt->stream_index]->codec;
>>> +    int keyframe = !!(pkt->flags & PKT_FLAG_KEY);
>>> +    int ret = 0;
>>> +
>>> +    if (codec->codec_type == AVMEDIA_TYPE_VIDEO && keyframe && mkv->cluster_pos) {
>>> +        // Start a new cluster when we get a key frame
>>> +        int64_t ts = mkv->tracks[pkt->stream_index].write_dts ? pkt->dts : pkt->pts;
>>> 
>>> +        av_log(s, AV_LOG_DEBUG, "Starting new cluster at offset %" PRIu64
>>> +          " bytes, pts %" PRIu64 "\n", url_ftell(pb), ts);
>>> +
>>> +        end_ebml_master(pb, mkv->cluster);
>>> +        mkv->cluster_pos = 0;
>>> +        if (mkv->dyn_bc)
>>> +            mkv_flush_dynbuf(s);
>>> +    }
>> 
>> There should be a minimum cluster size in addition to starting on keyframes.
> With the current setup it would pass back through the initial write
> and hit the worst case 30s.

You mean the first keyframe after reaching the min size wouldn't trigger this condition? At any rate, the idea is that if you have several keyframes to avoid starting a new cluster for each of them. The min size only has to be like 1-4k or so to avoid the worst of it.