On 07/15/2010 02:28 PM, Sebastian Vater wrote:
Vitor Sessak a écrit :
On 07/15/2010 12:18 AM, Sebastian Vater wrote:
So here is it, I merged them all into one to avoid flooding the ML.
They're getting small enough now, to merge them all into one, I think!
This is not a patch against latest SVN, this is a patch against your
SoC tree and thus unreadable (your patch should contain all the lines
of libavsequencer/* since none of this files exists now in main svn).
Fixed.
diff --git a/libavsequencer/track.h b/libavsequencer/track.h
index c1370ed..33095d9 100755
--- a/libavsequencer/track.h
+++ b/libavsequencer/track.h
@@ -34,12 +34,18 @@
* version bump.
*/
[...]
/** 0x20 - Set volume:
Again, is saying "0x20" in the comment relevant? If this value for the
set volume command is format-dependent (i.e., it is only 0x20 in MOD
but 0x55 in XM), this is misleading.
The demuxer/decoder will convert this to effect number 0x20 anyway, in
MOD the set volume command is C (internally = 0x02). The goal is to have
a unique command set independent from the origin file.
Data word consists of two 8-bit pairs named xx and yy
where xx is the volume level (ranging either from
@@ -756,7 +729,8 @@ typedef struct AVSequencerTrackEffect {
means that the tremolo envelope values will be inverted. */
#define AVSEQ_TRACK_EFFECT_CMD_T_TREMO_ONCE 0x2F
Wasn't you going to change these (and many other) to a enum? I
remember you said you would change it soon, but being afraid of
breaking other files that uses this header, but what code exactly does
replacing
Yes, will do this tonight.
#define AVSEQ_TRACK_EFFECT_CMD_T_TREMO_ONCE 0x2F
#define AVSEQ_TRACK_EFFECT_CMD_STOP_FX 0x1D
by
enum Whatever {
AVSEQ_TRACK_EFFECT_CMD_T_TREMO_ONCE = 0x2F,
AVSEQ_TRACK_EFFECT_CMD_STOP_FX = 0x1D,
}
breaks?
I access them like:
if (flags& AVSEQ_TRACK_EFFECT_CMD_STOP_FX) { .. }
Won't have change that to:
if (flags& Whatever.AVSEQ_TRACK_EFFECT_CMD_STOP_FX) { .. }
then?
No, at least in my box the following compiles fine:
enum Whatever {
AVSEQ_TRACK_EFFECT_CMD_T_TREMO_ONCE = 0x2F,
AVSEQ_TRACK_EFFECT_CMD_STOP_FX = 0x1D,
};
int f(int flags)
{
if (flags & AVSEQ_TRACK_EFFECT_CMD_T_TREMO_ONCE)
return 1;
else
return 0;
}
Finally, wasn't you going to remove all the defines for the default
values by creating a function that init them? Can you send a new patch
after applying these two major changes (ie, using enum instead of
defines and avoiding the defines for the default values)? Ideally, you
should send a new patch after all the feedback was already taken into
account...
-Vitor
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
