[FFmpeg-soc] [soc] libavsequencer [PATCH 01/08] Music module public API header file.

[Vitor Sessak]

On 08/07/2010 09:42 PM, Sebastian Vater wrote:
> Vitor Sessak a écrit :
>> On 07/15/2010 09:35 PM, Ronald S. Bultje wrote:
>>> Hi,
>>>
>>> On Thu, Jul 15, 2010 at 3:12 PM, Sebastian Vater
>>> <cdgs.basty-gM/Ye1E23mwN+BqQ9rBEUg at public.gmane.org>   wrote:
>>>> Just a question for understanding...what's the advantage of using
>>>> enum's
>>>> then? The only one I see straight away is that there is less space
>>>> required (saving 4 characters by replacing '#define ' with '    ').
>>>
>>> Documentation tools (e.g. doxy) will group them together and treat
>>> them as a group of related properties, rather than separate entities.
>>> That's a big one for a huge project such as FFmpeg, where in the
>>> future many people will look at and modify your code, without
>>> necessarily understanding everything at the beginning.
>>
>> Not to mention having variables of the type "enum Whatever" instead of
>> a plain int, which tell immediately which kind of constants should be
>> assigned to it.
>
> Hi I have excellent news!
>
> libavsequencer now flawlessly integrates into FFmpeg, just check out my
> latest git. Please do a git pull --rebase, Stefano had problems without
> using it.
>
> Here are the module.[ch] part of the BSS to review.
>
> This version compiles perfectly.
>

>
> diff --git a/libavsequencer/module.h b/libavsequencer/module.h
> new file mode 100755
> index 0000000..abd9792
> --- /dev/null
> +++ b/libavsequencer/module.h
> @@ -0,0 +1,108 @@
> +/*
> + * AVSequencer music module management
> + * Copyright (c) 2010 Sebastian Vater <cdgs.basty at googlemail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVSEQUENCER_MODULE_H
> +#define AVSEQUENCER_MODULE_H
> +
> +#include "libavutil/log.h"
> +#include "libavformat/avformat.h"
> +#include "libavsequencer/song.h"
> +#include "libavsequencer/instr.h"
> +
> +/**
> + * Sequencer module structure.
> + * New fields can be added to the end with minor version bumps.
> + * Removal, reordering and changes to existing fields require a major
> + * version bump.
> + */
> +typedef struct AVSequencerModule {
> +    /**
> +     * information on struct for av_log
> +     * - set by avseq_alloc_context
> +     */
> +    const AVClass *av_class;
> +
> +    /** Metadata information: Original module file name, module name,
> +     *  module message, artist, genre, album, begin and finish date of
> +     * composition and comment.  */
> +    AVMetadata *metadata;
> +
> +    /** Array (of size songs) of pointers containing every sub-song
> +       for this module.  */
> +    AVSequencerSong **song_list;
> +
> +    /** Number of sub-songs attached to this module.  */
> +    uint16_t songs;
> +
> +    /** Array (of size instruments) of pointers containing every
> +       instrument for this module.  */
> +    AVSequencerInstrument **instrument_list;
> +
> +    /** Number of instruments attached to this module.  */
> +    uint16_t instruments;
> +
> +    /** Array (of size envelopes) of pointers containing every
> +       evelope for this module.  */
> +    AVSequencerEnvelope **envelope_list;
> +
> +    /** Number of envelopes attached to this module.  */
> +    uint16_t envelopes;
> +
> +    /** Array (of size keyboards) of pointers containing every
> +       keyboard definition list for this module.  */
> +    AVSequencerKeyboard **keyboard_list;
> +
> +    /** Number of keyboard definitions attached to this module.  */
> +    uint16_t keyboards;
> +
> +    /** Array (of size arpeggios) of pointers containing every
> +       arpeggio envelope definition list for this module.  */
> +    AVSequencerArpeggio **arpeggio_list;
> +
> +    /** Number of arpeggio definitions attached to this module.  */
> +    uint16_t arpeggios;


> +    /** Forced duration of the module, in AV_TIME_BASE fractional
> +       seconds. This is the total sum of all sub-song durations
> +       this module contains or zero if the duration is unknown and
> +       also cannot be automatically determined. The composer then can
> +       set manually a duration after which the player can skip over to
> +       the next module if it is playing back in once mode.  */
> +    uint64_t forced_duration;

This looks like a mix between what we discussed and what it was before. 
For me (and you seemed to agree), if the song finishes after the last 
note, this field should be set to zero, no matter if the duration can be 
automatically determined or not. If the duration _can_ be determined 
automatically, one should set the corresponding field of 
_AVCodecContext_ (obviously, when forced_duration != 0, we can always 
determine the song duration and it is equal to forced_duration).

-Vitor