On 07/10/2010 11:31 PM, Sebastian Vater wrote:
Vitor Sessak a écrit :
On 07/10/2010 07:57 PM, Sebastian Vater wrote:
Vitor Sessak a écrit :
+#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);
Already fixed in github.
It is still not good in github. Why duplicating the
AVFormatContext->metadata as AVSenquencerSynth->metadata?
You surely meant AVSequencerModule->metadata not synth, right?
Oops, there is a file metadata inside the synth data? I hope some
demuxer expert can tell how to set metadata in these cases...
Since I imagine that adding new instruments, songs, etc should be
pretty rare and their number should be pretty small, I imagine that
the extra simplicity of using an array is worth the O(n) cost to add
an item.
I agree totally with that! O(1) for access is way more important than
inserting or deleting. What do you think about:
AVSequencerInstrument **instrument_list?
So we have just a pointer list to move around instead the huge structures?
I don't know what typical values are, but is the folowing doable?
#define MAX_INSTRUMENTS [... some value here ...]
AVSequencerInstrument *instrument_list[MAX_INSTRUMENTS];
Everything access and insertion O(1) and both done in one line of code...
If it is currently unused, I think the variable should be removed
altogether. We can add it later when needed.
Fixed locally right now (will commit soon) by removing both compat_flags
and flags, since compat_flags is also unused in the module right now.
Nice.
+ /** 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?
You mean like:
uint8_t **unknown_data?
Why not then a "void *"?
void ** or uint8_t ** would be better I think...each chunk is a pointer
then. That way single chunks can be changed without recalculating
everything else.
Why do you think in chunks? They might be organized in a tree inside the
file or a circularly linked list, or etc. void * do not exclude int8_t **...
Fixed in github by removing user_data in all header files.
However, these structures are all to be supposed to be writable by the
client, how you otherwise could write a tracker using FFmpeg, when the
stuff the user edits can't be applied to this structure?
I agree.
user_data is removed though, since rethinking this took me to the
conclusion that it's better to let the application developer decide this
and if he needs extra data for this, he can still do:
typedef struct MyTrackerModule {
AVSequencerModule module;
[...] custom stuff follows here with exact declaration
} MyTrackerModule;
Nice.
-Vitor
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
