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

[Dmitry Timoshkov]

Chris Robinson  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.

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.