On 5 Feb, Martin Ruckert wrote:
> --------- a draft of mpglib.h---------
> #include <stdlib.h>
> /* mpglib.h (draft) a new mpglib interface (draft)
> */
>
>
> typedef int mpg_id;
> /* A handle to a mpg stream.
> Values are created with mpg_open, used with mpg_read,
> and destroyed with mpg_close.
>
> Several mp3 streams might be open for decoding at the same time,
> representing e.g. several tracks of a mixing application. mpg_open
> will always return the smallest non-negative number that is not yet
> associated with an open stream. E.g opening five streams will return
> the numbers 0,1,2,3,4 in this order; then after closing stream 2,
> the next call to mpg_open will return 2. Handles can be used by an
> application as an index into an array containing application specific
> supplemental information.
> */
What about nailing this down more.
- Are you planing to allow negative numbers?
No -> unsigned.
- How many handles are you willing to handle?
If you make assumptions on the number of handles, you should use specific
types, e.g. int32_t or something like this.
What about having streams 1, 2 and 3 open, closing stream 2, and opening
another one:
- What will the next call to mpg_open return?
- Do we need to specify this? Can't this be opague to the user of the
API?
Generally I would like to see in LAME more types which specify the bit
width instead of some unspecific types (think about the use of it on 64
bit architectures, e.g. how large is an int or long on 32 bit systems,
and how large are they on 64 bit systems... we already have an
inconsistency in lame_encode_long(), please don't add more).
> /* Information on streams is provided in a mpg_info structure */
> typedef struct mpg_info {
> mpg_id self; /* the stream that provides the information */
>
> /* MPG format parameters */
> int version; /* can be one of */
> #define MPG_V2_5 0x00 /* MPEG Version 2.5 (later extension of MPEG 2) */
> #define MPG_V2_0 0x02 /* MPEG Version 2 (ISO/IEC 13818-3) */
> #define MPG_V1_0 0x03 /* MPEG Version 1 (ISO/IEC 11172-3) */
What about an enum instead?
> int layer; /* can be 1 2 or 3 */
> int crc_protected; /* can be TRUE or FALSE */
> int bitrate; /* the frame's bitrate */
> int framesize; /* the byte size of this frame */
> int mode; /* can be one of the following */
> #define MPG_MD_STEREO 0x00
> #define MPG_MD_JOINT_STEREO 0x01
> #define MPG_MD_DUAL_CHANNEL 0x02
> #define MPG_MD_MONO 0x03
enum? See lame.h.
> int mode_extension; /* can be 0,1,2, or 2 */
^ ^
| |
+--+--+
?
enum?
> int private; /* can be TRUE or FALSE */
> int copyright; /* can be TRUE or FALSE */
> int original; /* can be TRUE or FALSE */
3x 32 bit for something which can fit into 8 bit (bitfield)?
The same applies to crc_protected. And grouping those 4 together allows
further packing.
> int emphasis; /* can be 0,1,2, or 2 */
>
> /* PCM format parameters */
> int sample_frequency; /* the frequency in Hz */
unsigned?
> int channels; /* the number of channels 1=mono 2=stereo */
unsigned?
And if mpglib doesn't want to decode AAC sometimes in the future, maybe
'int8_t' would be enough.
With appropiate reordering of the structure you may minimize gaps.
> } mpg_info;
> extern mpg_id mpg_open(int next_byte(mpg_id self),
> int tag_handler(mpg_id self,
> int tag,
> int next_byte(mpg_id self),
> size_t size),
> mpg_options *p);
>
> /* Opens an mpg decoding stream.
> On failure it returns -1.
If you go the unsigned route above, '0' would be the failure number.
> The return value can be used in successive mpg_read and
> mpg_close calls.
>
> The parameters:
>
> next_byte is a function that will deliver the next unsigned char
> (0-255) cast to an int from the mp3 stream. It may return -1 on end
Why does it return an int when it is an unsigned char (I don't know much
about mpglib or the use of this part of the API)?
> of file or error. The parameter self can be used to identify the
> stream in case there are multiple open streams at the same time.
> In the simplest case this might, for example, be the
> function int next_byte(mpg_id self) { return getchar(); }.
>
> tag_handler is a function, that will be called if the decoding
> engine detects data in the stream that presents additional
> information (tags).
>
> tag_handler might be NULL, in this case tags are just skipped.
'Skipped' as in 'thrown away' or as in 'played as silence'?
> The tag_handler gets four parameters:
>
> self, the stream that found the tag. This parameter can be used to
> identify the stream that called the tag handler.
>
> tag, this might be one of the following:
> */
> #define MPG_TAG_UNKNOWN 0 /* the decoding engine has no idea */
> #define MPG_TAG_RIFF 1
> #define MPG_TAG_ID3V1 2
> #define MPG_TAG_ID3V2 3
> #define MPG_TAG_LYRICS 4
> #define MPG_TAG_AID 5
> #define MPG_TAG_XING 6
> #define MPG_TAG_LAME 7
enum?
> extern int mpg_close(mpg_id s);
>
> /*
> Will close the stream s. It returns 0 on success and a negative
> value if an error occurred. It will free all data associated with the
> stream. After closing it, the stream id's might not longer be used
> in a call to mpg_read() or mpg_info(). A later call of mpg_open
> might return again the same id for a new stream.
> */
BTW. do you plan to set errno in the failure case?
Bye,
Alexander.
--
Reboot America.
http://www.Leidinger.net Alexander @ Leidinger.net
GPG fingerprint = C518 BC70 E67F 143F BE91 3365 79E2 9C60 B006 3FE7
_______________________________________________
mp3encoder mailing list
[EMAIL PROTECTED]
http://minnie.tuhs.org/mailman/listinfo/mp3encoder
