Re: [PATCH] include : Add dvdif.idl

[Zebediah Figura]

On 4/9/20 10:23 AM, Vijay Kiran Kamuju wrote:
> +typedef enum
> +{
> - DVD_ResetOnStop                     = 1,
> - DVD_NotifyParentalLevelChange       = 2,
> - DVD_HMSF_TimeCodeEvents             = 3,
> - DVD_AudioDuringFFwdRew              = 4,
> - DVD_EnableNonblockingAPIs           = 5,
> - DVD_CacheSizeInMB                   = 6,
> - DVD_EnablePortableBookmarks         = 7,
> - DVD_EnableExtendedCopyProtectErrors = 8,
> - DVD_NotifyPositionChange            = 9,
> - DVD_IncreaseOutputControl           = 10,
> - DVD_EnableStreaming                 = 11,
> - DVD_EnableESOutput 	                = 12,

Stray spaces here.
> - DVD_EnableTitleLength               = 13,
> - DVD_DisableStillThrottle            = 14,
> - DVD_EnableLoggingEvents             = 15,
> - DVD_MaxReadBurstInKB                = 16,
> - DVD_ReadBurstPeriodInMS             = 17,
> - DVD_RestartDisc                     = 18,
> - DVD_EnableCC                        = 19
>
> +} DVD_OPTION_FLAG;

...
> +cpp_quote("#include <pshpack1.h>")
> +typedef struct tagDVD_AudioAttributes
> +{
> - DVD_AUDIO_APPMODE AppMode;
> - BYTE  AppModeData;
> - DVD_AUDIO_FORMAT AudioFormat;
> - LCID  Language;
> - DVD_AUDIO_LANG_EXT LanguageExtension;
> - BOOL  fHasMultichannelInfo;
> - DWORD dwFrequency;
> - BYTE  bQuantization;
> - BYTE  bNumberOfChannels;
> - DWORD dwReserved[2];
>
> +} DVD_AudioAttributes;
> -
>
> +typedef struct tagDVD_KaraokeAttributes
> +{
> - BYTE bVersion;
> - BOOL fMasterOfCeremoniesInGuideVocal1;
> - BOOL fDuet;
> - DVD_KARAOKE_ASSIGNMENT ChannelAssignment;
> - WORD wChannelContents[8];
>
> +} DVD_KaraokeAttributes;
> -
>
> +typedef struct tagDVD_TitleMainAttributes
> +{
> - union
> - {
> -
>    DVD_TITLE_APPMODE AppMode;
> -
>    DVD_HMSF_TIMECODE TitleLength;
> - } DUMMYUNIONNAME;
> -
> - DVD_VideoAttributes VideoAttributes;
> - ULONG ulNumberOfAudioStreams;
> - DVD_AudioAttributes  AudioAttributes[8];
> - DVD_MultichannelAudioAttributes MultichannelAudioAttributes[8];
> - ULONG ulNumberOfSubpictureStreams;
> - DVD_SubpictureAttributes SubpictureAttributes[32];
>
> +} DVD_TitleAttributes;
> +cpp_quote("#include <poppack.h>")

I don't see any packing directives in Microsoft's headers for these
structures. In fact, the Windows 10 SDK includes directives to ignore
padding warnings.

...
> +typedef struct tagDVD_DECODER_CAPS
> +{
> - DWORD  dwSize;
> - DWORD  dwAudioCaps;
> - double dFwdMaxRateVideo;
> - double dFwdMaxRateAudio;
> - double dFwdMaxRateSP;
> - double dBwdMaxRateVideo;
> - double dBwdMaxRateAudio;
> - double dBwdMaxRateSP;
> - DWORD  dwRes1 ;
> - DWORD  dwRes2 ;

Stray spaces.
> - DWORD  dwRes3;
> - DWORD  dwRes4;
>
> +} DVD_DECODER_CAPS;

...
> +[
> - local,
> - object,
> - uuid(a70efe61-e2a3-11d0-a9be-00aa0061be93),
> - pointer_default(unique)

Can you please keep indentation at a consistent 4 spaces?
> +]
> +interface IDvdControl : IUnknown
> +{
> - HRESULT TitlePlay(
> -
>  [in] ULONG ulTitle);

Please avoid Hungarian notation for parameter names outside of library
blocks. There's also no need to put each parameter on a separate line,
or to put spaces between each function.
> -
> - HRESULT ChapterPlay(
> -
>  [in] ULONG ulTitle,
> -
>  [in] ULONG ulChapter);

...
> - HRESULT GetRoot(
> -
>  [out, size_is(ulBufSize)] LPSTR *pRoot,

This isn't a (double) pointer in my copy of the PSDK.
> -
>  [in] ULONG ulBufSize,
> -
>  [out] ULONG *pulActualSize);
>
> +}

...
> - HRESULT GetCmdFromEvent(
> -
>  [in] ULONG_PTR lParam1,

This is LONG_PTR in my copy of the PSDK.
> -
>  [out] IDvdCmd **pCmdObj);
> -
> - HRESULT GetDefaultMenuLanguage(
> -
>  [out] LCID *pLanguage);
> -
> - HRESULT GetDefaultAudioLanguage(
> -
>  [out] LCID *pLanguage,
> -
>  [out] DVD_AUDIO_LANG_EXT *pAudioExtension);
> -
> - HRESULT SelectDefaultSubpictureLanguage(
> -
>  [out] LCID *pLanguage,
> -
>  [out] DVD_SUBPICTURE_LANG_EXT *pSubpictureExtension);
> -
> - HRESULT GetDecoderCaps(
> -
>  [out] DVD_DECODER_CAPS *pCaps);
> -
> - HRESULT GetButtonRect(
> -
>  [in] ULONG ulButton,
> -
>  [out] RECT *pRect);
> -
> - HRESULT IsSubpictureStreamEnabled(
> -
>  [in] ULONG ulStreamNum,
> -
>  [out] BOOL *pbEnabled);
>
> +}
> -
>
> +[
> +local,
> +object,
> +uuid(fcc152B6-f372-11d0-8e00-00c04fd7c08b),
> +pointer_default(unique)

Please indent these attributes, and avoid mixing case in GUID declarations.
> +]
> +interface IDvdGraphBuilder : IUnknown
> +{
> - HRESULT GetFiltergraph(
> -
>  [out] IGraphBuilder **ppGB);
> -
> - HRESULT GetDvdInterface(
> -
>  [in] REFIID riid,
> -
>  [out] void **ppIF);
> -
> - HRESULT RenderDvdVideoVolume(
> -
>  [in] LPCWSTR lpcwszPathName,
> -
>  [in] DWORD dwFlags,
> -
>  [out] AM_DVD_RENDERSTATUS *pStatus);
>
> +}
> diff --git a/include/strmif.idl b/include/strmif.idl
> index a06f99b751..38af866f47 100644
> --- a/include/strmif.idl
> +++ b/include/strmif.idl
> @@ -27,5 +27,6 @@ import "objidl.idl";
>
> #include <dyngraph.idl>
>
> -/* TODO: #include <dvdif.h> */
>  #include <vmrender.idl>
> -
>
> +#include <dvdif.idl>
>
> -- 
> 2.26.0