On 08/07/2010 09:54 PM, Sebastian Vater wrote:
Sebastian Vater a écrit :
Vitor Sessak a écrit :

On 07/13/2010 11:13 PM, Sebastian Vater wrote:


Original TuComposer didn't even use typedef's at all, the only way to
access them was a struct TuComposerInstrEnvelope, etc.

I also prefer the way to provide the target programmer more freedom than
shrinking it, despite the fact that writing the header the way I did
doesn't require much amount of time.

This, however, is just a target-programmer-user-friendly purpose and
wouldn't interfere with player.c by changing that, since I replaced all
struct AVSequencer* with simply AVSequencer*

However, changing that again, would require extra work by additionally
making it a bit uncomfortable to target programmers. So if you are not
piecy on this, I would keep is it at now, I will leave that decision
completely to you, though. If you want me to remove that, I'll do.

Leave it as is. Maybe it is just my personal taste and not very
important ATM.


Ok. Updated patch though.


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 instr.[ch] part of the BSS to review.

This version compiles perfectly.

diff --git a/libavsequencer/instr.h b/libavsequencer/instr.h
new file mode 100755
index 0000000..1a1eede
--- /dev/null
+++ b/libavsequencer/instr.h
@@ -0,0 +1,571 @@
+/*
+ * AVSequencer instrument management
+ * Copyright (c) 2010 Sebastian Vater <[email protected]>
+ *
+ * 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_INSTR_H
+#define AVSEQUENCER_INSTR_H
+
+#include "libavutil/log.h"
+#include "libavformat/avformat.h"
+
+/** AVSequencerEnvelope->flags bitfield.  */
+enum AVSequencerEnvelopeFlags {
+    AVSEQ_ENVELOPE_LOOP             = 0x0001, ///< Envelope uses loop nodes
+    AVSEQ_ENVELOPE_SUSTAIN          = 0x0002, ///< Envelope uses sustain nodes

Why do you have to set a flag saying if it uses or not some feature? Can't this be evaluated from the other fields?

+    AVSEQ_ENVELOPE_PINGPONG         = 0x0004, ///< Envelope loop is in ping 
pong mode
+    AVSEQ_ENVELOPE_SUSTAIN_PINGPONG = 0x0008, ///< Envelope sustain loop is in 
ping pong mode
+};
+/**
+ * Envelope structure used by instruments to apply volume / panning
+ * or pitch manipulation according to an user defined waveform.
+ * 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 AVSequencerEnvelope {
+    /**
+     * information on struct for av_log
+     * - set by avseq_alloc_context
+     */
+    const AVClass *av_class;
+
+    /** Metadata information: Original envelope file name, envelope
+     *  name, artist and comment.  */
+    AVMetadata *metadata;
+
+    /** The actual node data of this envelope as signed 16-bit integer.
+       For a volume envelope, we have a default scale range of -32767
+       to +32767, for panning envelopes the scale range is between -8191
+       to +8191. For slide, vibrato, tremolo, pannolo (and their auto
+       versions), the scale range is between -256 to +256.  */
+    int16_t *data;

+    /** The node points values or 0 if the envelope is empty.  */
+    uint16_t *node_points;

What is the point of an empty envelope?

+    /** Number of dragable nodes of this envelope (defaults to 12).  */
+    uint16_t nodes;
+
+    /** Number of envelope points, i.e. node data values which
+       defaults to 64.  */
+    uint16_t points;
+
+    /** Instrument envelope flags. Some sequencers feature
+       loop points of various kinds, which have to be taken
+       care specially in the internal playback engine.  */

+    uint16_t flags;

enum...

+    /** Envelope tempo in ticks (defaults to 1, i.e. change envelope
+       at every frame / tick).  */
+    uint16_t tempo;
+
+    /** Envelope sustain loop start point.  */
+    uint16_t sustain_start;
+
+    /** Envelope sustain loop end point.  */
+    uint16_t sustain_end;
+
+    /** Envelope sustain loop repeat counter for loop range.  */
+    uint16_t sustain_count;
+
+    /** Envelope loop repeat start point.  */
+    uint16_t loop_start;
+
+    /** Envelope loop repeat end point.  */
+    uint16_t loop_end;
+
+    /** Envelope loop repeat counter for loop range.  */
+    uint16_t loop_count;
+
+    /** Randomized lowest value allowed.  */
+    int16_t value_min;
+
+    /** Randomized highest value allowed.  */
+    int16_t value_max;
+
+    /** Array of pointers containing every unknown data field where
+       the last element is indicated by a NULL pointer reference. The
+       first 64-bit of the unknown data contains an unique identifier
+       for this chunk and the second 64-bit data is actual unsigned
+       length of the following raw 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 envelopes in those
+       chunks, which then won't get lost in that case.  */
+    uint8_t **unknown_data;
+} AVSequencerEnvelope;
+
+/**
+ * Keyboard definitions structure used by instruments to map
+ * note to samples. C-0 is first key. B-9 is 120th key.
+ * 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 AVSequencerKeyboard {
+    /**
+     * information on struct for av_log
+     * - set by avseq_alloc_context
+     */
+    const AVClass *av_class;
+
+    /** Metadata information: Original keyboard file name, keyboard
+     *  name, artist and comment.  */
+    AVMetadata *metadata;

+    struct AVSequencerKeyboardEntry {
+        /** Sample number for this keyboard note.  */
+        uint16_t sample;
+
+        /** Octave value for this keyboard note.  */
+        uint8_t octave;
+
+        /** Note value for this keyboard note.  */
+        uint8_t note;
+    } key[120];

Why is this needed if you set already C-0 as the first key and B-9 as the 120th? Can't you just evaluate all that from the key index?

+} AVSequencerKeyboard;
+
+/**
+ * Arpeggio data structure, This structure is actually for one tick
+ * and therefore actually pointed as an array with the amount of
+ * different ticks handled by the arpeggio control.
+ * 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 AVSequencerArpeggioData {
+    /** Packed note or 0 if this is an arpeggio note.  */
+    uint8_t tone;
+
+    /** Transpose for this arpeggio tick.  */
+    int8_t transpose;
+
+    /** Instrument number to switch to or 0 for original instrument.  */
+    uint16_t instrument;
+
+    /** The four effect command bytes which are executed.  */
+    uint8_t command[4];
+
+    /** The four data word values of the four effect command bytes.  */
+    uint16_t data[4];
+} AVSequencerArpeggioData;


+/** AVSequencerArpeggio->flags bitfield.  */
+enum AVSequencerArpeggioFlags {
+    AVSEQ_ARPEGGIO_FLAG_LOOP                = 0x0001, ///< Arpeggio control is 
looped
+    AVSEQ_ARPEGGIO_FLAG_SUSTAIN             = 0x0002, ///< Arpeggio control 
has a sustain loop
+    AVSEQ_ARPEGGIO_FLAG_PINGPONG            = 0x0004, ///< Arpeggio control 
will be looped in ping pong mpde
+    AVSEQ_ARPEGGIO_FLAG_SUSTAIN_PINGPONG    = 0x0008, ///< Arpeggio control 
will have sustain loop ping pong mode enabled
+};

This looks duplicated.

+/**
+ * Arpeggio control envelope used by all instrumental stuff.
+ * 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 AVSequencerArpeggio {
+    /**
+     * information on struct for av_log
+     * - set by avseq_alloc_context
+     */
+    const AVClass *av_class;
+
+    /** Metadata information: Original arpeggio file name, arpeggio
+     *  name, artist and comment.  */
+    AVMetadata *metadata;
+
+    /** AVSequencerArpeggioData pointer to arpeggio data structure.  */
+    AVSequencerArpeggioData *data;
+
+    /** Instrument arpeggio control flags. Some sequencers feature

+       customized arpeggio command control.which have to be taken

s/control.which/control, which/
Also, enum.

-Vitor
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc

Reply via email to