Re: quartz: Partially implement the MPEG 1 Stream Splitter filter

[Dmitry Timoshkov]

Chris Robinson <chris.kcat(a)gmail.com> wrote:
> +static inline void flip_dword(DWORD *pVal)
> +{
> +    DWORD val = *pVal;
> +    val = (val>>24) | ((val>>8)&0xFF00) |
> +          ((val&0xFF00)<<8) | (val<<24);
> +    *pVal = val;
> +}
> +
> +static inline void flip_word(WORD *pVal)
> +{
> +    WORD val = *pVal;
> +    val = (val>>8) | (val<<8);
> +    *pVal = val;
> +}

You need to take WORDS_BIGENDIAN macro into account, not blindly assume that
we are running on a LE machine. Something like (as done in other DLLs):

#ifdef WORDS_BIGENDIAN
#define GET_BE_WORD(x) (x)
#define GET_BE_DWORD(x) (x)
#else
#define GET_BE_WORD(x) RtlUshortByteSwap(x)
#define GET_BE_DWORD(x) RtlUlongByteSwap(x)
#endif

Besides it looks like flip_word() is not used.
> +static int head_check(DWORD head)
> +{
> +    /* If this is a possible start code, check for a system or video header */
> +    if ((head & 0xFFFFFF00) == 0x100) {
> +        /* Check if we got a system or elementary stream start code */
> +        if (head == PACK_START_CODE || head == VIDEO_ELEMENTARY_STREAM ||
> +            head == AUDIO_ELEMENTARY_STREAM)
> +            return MPEG_SYSTEM_HEADER;
> +
> +        /* Check for a MPEG video sequence start code */
> +        if (head == SEQUENCE_HEADER_CODE)
> +            return MPEG_VIDEO_HEADER;
> +    }
> +
> +    /* This should give a good guess if we have an MPEG audio header */
> +    if((head & 0xffe00000) == 0xffe00000 && ((head>>17)&3) != 0x0 &&
> +       ((head>>12)&0xf) != 0xf && ((head>>10)&0x3) != 0x3)
> +        return MPEG_AUDIO_HEADER;

Why not use a struct with bit fields to simplify the above check?
> +    if (headercode & (1<<20))
> +    {
> +        lsf = ((headercode & (1<<19)) ? 0x0 : 0x1);
> +        mpg25 = 0;
> +    }
> +    else
> +    {
> +        lsf = 1;
> +        mpg25 = 1;
> +    }
> +
> +    layer         = 4-((headercode>>17)&0x3);
> +    bitrate_index =   ((headercode>>12)&0xf);
> +    freq_index    =   ((headercode>>10)&0x3) + (mpg25?6:(lsf*3));
> +    padding       =   ((headercode>> 9)&0x1);
> +    mode          =   ((headercode>> 6)&0x3);
> +    mode_ext      =   ((headercode>> 4)&0x3);
> +    emphasis      =   ((headercode    )&0x3);

Same here.

-- 
Dmitry.

quartz: Partially implement the MPEG 1 Stream Splitter filter

[Chris Robinson]

On Sunday 15 April 2007 08:33:12 pm you wrote:
> You need to take WORDS_BIGENDIAN macro into account, not blindly assume
> that we are running on a LE machine. Something like (as done in other
> DLLs):
>
> #ifdef WORDS_BIGENDIAN
> #define GET_BE_WORD(x) (x)
> #define GET_BE_DWORD(x) (x)
> #else
> #define GET_BE_WORD(x) RtlUshortByteSwap(x)
> #define GET_BE_DWORD(x) RtlUlongByteSwap(x)
> #endif

I was under the impression we didn't need to worry about architecture issues 
like that?
> Why not use a struct with bit fields to simplify the above check?

I'm not too familiar with how bit-fields work, and given the above, how they 
behave with endian differences. I'll play around with it and see what I can 
do, though.