[PATCH] include : Add dvdif.idl

[Vijay Kiran Kamuju]

Signed-off-by: Vijay Kiran Kamuju

[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

[Vijay Kiran Kamuju]

On Thu, Apr 9, 2020 at 6:13 PM Zebediah Figura  wrote:
> 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.

I removed them, it seems I updated on top of older commit.
Will fix it.
> > - 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.

Oh I assumed #pragma warning(push) to be packing directives.
Will fix it.
> ...
> > +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.

I removed them, it seems I updated on top of older commit.
Will fix it.
> > - 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?

Will fix it.
> > +]
> > +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.

I used the same notation as in PSDK, which is used in other idl files.
I used some older idl files as reference.
I can put all the parameters in the same line.
> > -
> > - 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.

Oops a typo. will correct it.
> > -
> >  [in] ULONG ulBufSize,
> > -
> >  [out] ULONG *pulActualSize);
> >
> > +}
>
> ...
> > - HRESULT GetCmdFromEvent(
> > -
> >  [in] ULONG_PTR lParam1,
>
> This is LONG_PTR in my copy of the PSDK.

Oops a typo. will correct it.
> > -
> >  [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.

I missed it, while trying to fix compile issues.
Will fix it.
> > +]
> > +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

[Zebediah Figura]

On 4/9/20 1:35 PM, Vijay Kiran Kamuju wrote:
> On Thu, Apr 9, 2020 at 6:13 PM Zebediah Figura  wrote:
> > 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.
>
> I removed them, it seems I updated on top of older commit.
> Will fix it.
> > > - 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.
>
> Oh I assumed #pragma warning(push) to be packing directives.
> Will fix it.
> > ...
> > > +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.
>
> I removed them, it seems I updated on top of older commit.
> Will fix it.
> > > - 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?
>
> Will fix it.
> > > +]
> > > +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.
>
> I used the same notation as in PSDK, which is used in other idl files.
> I used some older idl files as reference.
> I can put all the parameters in the same line.

Existing Wine IDLs often use identical parameter names to Microsoft's
headers. Inside of library declarations this is necessary, since those
names get encoded into type libraries. Outside of that, it's not
necessary, and if it's done in existing headers it's for no good reason.
I find such parameter names considerably uglier, and we generally want
to avoid mimickign Microsoft where not necessary.
> > > -
> > > - 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.
>
> Oops a typo. will correct it.
> > > -
> > >  [in] ULONG ulBufSize,
> > > -
> > >  [out] ULONG *pulActualSize);
> > >
> > > +}
> >
> > ...
> > > - HRESULT GetCmdFromEvent(
> > > -
> > >  [in] ULONG_PTR lParam1,
> >
> > This is LONG_PTR in my copy of the PSDK.
>
> Oops a typo. will correct it.
> > > -
> > >  [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.
>
> I missed it, while trying to fix compile issues.
> Will fix it.
> > > +]
> > > +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