[FFmpeg-devel] [PATCH] WMAPRO packet parser

[Michael Niedermayer]

On Thu, Aug 14, 2008 at 08:16:38PM +0200, Sascha Sommer wrote:
> Hi,
> 
> attached patch adds support for a wma3 decoder skeleton and packet parser.
> Basically less than what I expected to do during SOC but before I can continue 
> with the bitstream parsing after SOC I will first have to reverse engineer 
> and understand more of the inner parts of the codec.

well, if you do not have a functional decoder then i think there is only
limited sense in commiting any of this into svn ...
anyway, review below, also please see wma/wmadec.c the likely existing
similarities could help reverse engeneering and understanding. And factorizing
common code out would be required anyway ...


[...]

> +typedef struct {
> +    uint8_t num_subframes;     //< number of subframes for the current channel
> +    uint16_t subframe_len[MAX_SUBFRAMES]; //< subframe len in samples
> +    uint16_t channel_len;                 //< channel len in samples
> +} wma_channel_t;

vertical align of comments


> +
> +
> +/**
> + *@brief main decoder context
> + */
> +typedef struct WMA3DecodeContext {
> +    AVCodecContext*  avctx;           //< codec context for av_log
> +    GetBitContext    gb;              //< getbitcontext for the packet
> +    int              buf_bit_size;    //< buffer size in bits
> +
> +    /** Packet info */
> +    uint8_t          packet_sequence_number; //< current packet nr
> +    uint8_t          bit5;                   //< padding bit? (cbr files)
> +    uint8_t          bit6;
> +    uint8_t          packet_loss;            //< set in case of bitstream error
> +
> +    /** Stream info */
> +    uint16_t         samples_per_frame;     //< nr of outputed samples
> +    uint8_t          log2_block_align;      //< block align bits
> +    uint8_t          log2_block_align_bits; //< nr bits for block align len
> +    uint16_t         log2_frame_size;       //< frame size
> +    uint8_t          lossless;              //< lossless mode
> +    uint8_t          no_tiling;             //< frames are split in subframes
> +    int8_t           nb_channels;           //< nr of channels
> +    wma_channel_t    channel[MAX_CHANNELS]; //< per channel data

doxygen has special tags for comments covering a block @{ @} IIRC


> +
> +    /** Extradata */
> +    unsigned int     decode_flags;          //< used compression features
> +    unsigned int     dwChannelMask;
> +    uint8_t          sample_bit_depth;      //< bits per sample
> +
> +    /** General frame info */
> +    unsigned int     frame_num;             //< current frame number
> +    uint8_t          len_prefix;            //< frame is prefixed with its len
> +    uint8_t          allow_subframes;       //< frames may contain subframes
> +    uint8_t          max_num_subframes;     //< maximum number of subframes
> +    uint16_t         min_samples_per_subframe; //< minimum samples per subframe
> +    uint8_t          dynamic_range_compression;//< frame contains drc data
> +    uint8_t          drc_gain;                 //< gain for the drc tool
> +    uint8_t          update_samples_per_frame; //< recalculate output size
> +
> +
> +    /** Buffered frame data */
> +    int              prev_frame_bit_size;      //< saved number of bits
> +    uint8_t*         prev_frame;               //< prev frame data
> +
> +} WMA3DecodeContext;

> +
> +/**
> + *@brief helper function to print the most important members of the context
> + *@param s context
> + */
> +static void dump_context(WMA3DecodeContext *s)
> +{
> +#define PRINT(a,b) av_log(NULL,AV_LOG_ERROR," %s = %d\n", a, b);
> +#define PRINT_HEX(a,b) av_log(NULL,AV_LOG_ERROR," %s = %x\n", a, b);

NULL is not a good choice as context


[...]
> +/**
> + *@brief Get the samples per frame for this stream
> + *@param sample_rate output sample_rate
> + *@param decode_flags codec compression features
> + *@return number of output samples per frame
> + */
> +static int get_samples_per_frame(int sample_rate, unsigned int decode_flags) {
> +
> +    int samples_per_frame;
> +    int tmp;
> +
> +    if (sample_rate <= 16000)
> +        samples_per_frame = 512;
> +    else if (sample_rate <= 22050)
> +        samples_per_frame = 1024;
> +    else if (sample_rate <= 48000)
> +        samples_per_frame = 2048;
> +    else if (sample_rate <= 96000)
> +        samples_per_frame = 4096;
> +    else
> +        samples_per_frame = 8192;

this can be merged with wma.c


[...]


> +    /** Generic init */
> +    s->packet_loss = 0;
> +    s->log2_block_align = av_log2(avctx->block_align);

> +    s->log2_block_align_bits = av_log2(avctx->block_align*8);

hmm, that looks rather useless


> +    s->log2_frame_size = s->log2_block_align_bits + 1;

that as well


[...]
> +    while(more_frames && remaining_bits(s) > s->log2_frame_size){
> +        int frame_size = show_bits(&s->gb, s->log2_frame_size);
> +
> +        /** there is enough data for a full frame */
> +        if(remaining_bits(s) >= frame_size){
> +            /** decode the frame */
> +            more_frames = wma_decode_frame(s,&s->gb);
> +
> +            if(!more_frames){
> +                av_log(avctx, AV_LOG_ERROR, "no more frames\n");
> +            }
> +        }else
> +            more_frames = 0;
> +    }

this reminds me of the first loop in wma_decode_frame()


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080814/9eedfb24/attachment.pgp>