Hi,
Some comments (in this file and in a few others)...
On 07/07/2010 10:46 PM, Sebastian Vater wrote:
diff --git a/libavsequencer/module.h b/libavsequencer/module.h
new file mode 100755
index 0000000..af8fa89
[...]
+#include "libavsequencer/avsequencer.h"
+#include "libavsequencer/player.h"
+#include "libavutil/tree.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 {
+ /** Metadata information: Original module file name, module name,
+ * module message, artist, genre, album, begin and finish date of
+ * composition and comment. */
+ AVSequencerMetadata *metadata;
I don't think you need any struct for the metadata: AVFormatContext has
one already. Note that our metadata API allows you to add any name for
metadata by adding for ex.
av_metadata_set2(&s->metadata, "module message", module_message,
AV_METADATA_DONT_STRDUP_VAL);
+ /** AVSequencerPlayerChannel pointer to virtual channel data. */
+ AVSequencerPlayerChannel *channel_data;
+
+ /** Integer indexed tree root of sub-songs available in
+ this module with AVTreeNode->elem being a AVSequencerSong. */
+ AVTreeNode *song_list;
Why don't you change the numbering of the sub-songs so they are
sequential and then you can use a plain array instead of a tree?
Same for the other trees.
+ /** Integer indexed tree root of instruments available for
+ the whole module (shared between all sub-songs) with
+ AVTreeNode->elem being a AVSequencerInstrument. */
+ AVTreeNode *instrument_list;
+
+ /** Integer indexed tree root of envelopes used by module
+ with AVTreeNode->elem being a AVSequencerEnvelope.
+ There can be vibrato, tremolo, panbrello, spenolo,
+ volume, panning, pitch, envelopes, a resonance filter and
+ finally the auto vibrato, tremolo and panbrello envelopes. */
+ AVTreeNode *envelopes_list;
+
+ /** Integer indexed tree root of keyboard definitions
+ with AVTreeNode->elem being a AVSequencerKeyboard.
+ A keyboard definition maps an instrument octave/note-pair
+ to the sample number being played. */
+ AVTreeNode *keyboard_list;
+
+ /** Integer indexed tree root of arpeggio envelopes
+ with AVTreeNode->elem being a AVSequencerArpeggioEnvelope.
+ Arpeggio envelopes allow to fully customize the arpeggio
+ command by playing the envelope instead of only a
+ repetive set of 3 different notes. */
+ AVTreeNode *arp_env_list;
+
+ /** Duration of the module, in AV_TIME_BASE fractional
+ seconds. This is the total sum of all sub-song durations
+ this module contains. */
+ uint64_t duration;
+
+ /** Maximum number of virtual channels, including NNA (New Note
+ Action) background channels to be allocated and processed by
+ the mixing engine (defaults to 64). */
+ uint16_t channels;
+#define AVSEQ_MODULE_CHANNELS 64
+
+ /** Compatibility flags for playback. There are rare cases
+ where effect handling can not be mapped into internal
+ playback engine and have to be handled specially. For
+ each module which needs this, this will define new
+ flags which tag the player to handle it to that special
+ way. */
+ int8_t compat_flags;
+
+ /** Module playback flags. */
+ int8_t flags;
The comment can be removed, it don't add any information that is not
either in the struct name or the var name.
+ /** 64-bit integer indexed unique key tree root of unknown data
+ fields for input file reading with AVTreeNode->elem being
+ unsigned 8-bit integer data. Some formats are chunk based
+ and can store information, which can't be handled by some
+ other, in case of a transition the unknown data is kept as
+ is. Some programs write editor settings for module in those
+ chunks, which won't get lost then. The first 8 bytes of this
+ data is an unsigned 64-bit integer length in bytes of
+ the unknown data. */
+ AVTreeNode *unknown_data;
Might make sense storing it somewhere, but why not a plain buffer?
+ /** This is just a data field where the user solely
+ decides, what the usage (if any) will be. */
+ uint8_t *user_data;
This struct is not supposed to be writable by the client, so I don't
think this field make sense. This is also inconsistent with the rest of
libav*.
+} AVSequencerModule;
+
+/**
+ * Registers a module to the AVSequencer.
+ *
+ * @param module the AVSequencerModule to be registered
+ * @return >= 0 on success, error code otherwise
+ *
+ * @NOTE This is part of the new sequencer API which is still under
construction.
+ * Thus do not use this yet. It may change at any time, do not expect
+ * ABI compatibility yet!
+ */
+int avseq_module_register(AVSequencerModule *module);
?
Should this be created only when the file is opened? Normally, we have
xxx_register() only for things that are statically initialized before
opening any file (codecs, demuxers, etc).
-Vitor
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
