Re: [PATCH v2] dmime: Parse TimeSig track data.

[Michael Stefaniuc]

On 7/31/20 2:32 AM, Alistair Leslie-Hughes wrote:
> The tims data actually has a DWORD (size) before the structure.

That's because it is not one item but an array of items. This patch will
read only the first element.

I've seen that array pattern in the Tempo Track.
I have a cleanup patch for the Tempo Track but I'm not yet with the
patch. I have a structure too with an array[1] as last element, but I
need to pack the structure.
I have attached the patch so you can have a look at my implementation in
tempo_IPersistStream_Load().

Though I will redo my patch by exporting stream_read() from dmobject.c
and use that to first read the size DWORD and then just stream_read()
the array avoiding the variable size struct. Of course I'll still keep
the validation that the advertised size is a multiple of sizeof(item).

bye
    michael
> Signed-off-by: Alistair Leslie-Hughes
>
> dlls/dmime/timesigtrack.c | 61 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/dmime/timesigtrack.c b/dlls/dmime/timesigtrack.c
> index 697b3e3593b..f61e87806e9 100644
> --- a/dlls/dmime/timesigtrack.c
> +++ b/dlls/dmime/timesigtrack.c
> @@ -37,6 +37,11 @@ static inline IDirectMusicTimeSigTrack *impl_from_IDirectMusicTrack(IDirectMusic
>      return CONTAINING_RECORD(iface, IDirectMusicTimeSigTrack, IDirectMusicTrack_iface);
>  }
>
> +static inline IDirectMusicTimeSigTrack *impl_from_IPersistStream(IPersistStream *iface)
> +{
> - return CONTAINING_RECORD(iface, IDirectMusicTimeSigTrack, dmobj.IPersistStream_iface);
>
> +}
> -
>
> static HRESULT WINAPI IDirectMusicTrackImpl_QueryInterface(IDirectMusicTrack *iface, REFIID riid,
>          void **ret_iface)
>  {
> @@ -207,10 +212,62 @@ static const IDirectMusicTrackVtbl dmtack_vtbl = {
>      IDirectMusicTrackImpl_Clone
>  };
>
> +struct timesig_item
> +{
> - DWORD size;
> - DMUS_IO_TIMESIGNATURE_ITEM timesig;
>
> +};
> -
>
> +static HRESULT parse_timetrack_list(IDirectMusicTimeSigTrack *This, IStream *stream,
> -
>        struct chunk_entry *timesig)
>
> +{
> - HRESULT hr;
> - struct chunk_entry chunk = {.parent = timesig};
> - struct timesig_item item;
> -
> - TRACE("Parsing segment form in %p: %s\n", stream, debugstr_chunk(timesig));
> -
> - while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) {
> -
>    if (chunk.id != DMUS_FOURCC_TIMESIGNATURE_TRACK)
> -
>        return DMUS_E_UNSUPPORTED_STREAM;
> -
> -
>    if (FAILED(hr = stream_chunk_get_data(stream, &chunk, &item, sizeof(item)))) {
> -
>        WARN("Failed to read data of %s\n", debugstr_chunk(&chunk));
> -
>        return hr;
> -
>    }
> -
> -
>    TRACE("  Size %d\n", item.size);
> -
> -
>    TRACE("Found DMUS_IO_TIMESIGNATURE_ITEM\n");
> -
>    TRACE("  - lTime %d\n", item.timesig.lTime);
> -
>    TRACE("  - bBeatsPerMeasure %d\n", item.timesig.bBeatsPerMeasure);
> -
>    TRACE("  - bBeat %d\n", item.timesig.bBeat);
> -
>    TRACE("  - wGridsPerBeat %d\n", item.timesig.wGridsPerBeat);
> - }
> -
> - return SUCCEEDED(hr) ? S_OK : hr;
>
> +}
> -
>
> static HRESULT WINAPI time_IPersistStream_Load(IPersistStream *iface, IStream *stream)
>  {
> - FIXME(": Loading not implemented yet\n");
> - return S_OK;
> - IDirectMusicTimeSigTrack *This = impl_from_IPersistStream(iface);
> - HRESULT hr;
> - struct chunk_entry chunk = {0};
> -
> - TRACE("%p, %p\n", This, stream);
> -
> - if (!stream)
> -
>    return E_POINTER;
> -
> - if ((hr = stream_get_chunk(stream, &chunk) != S_OK))
> -
>    return hr;
> -
> - if (chunk.id == FOURCC_LIST && chunk.type == DMUS_FOURCC_TIMESIGTRACK_LIST)
> -
>    hr = parse_timetrack_list(This, stream, &chunk);
> - else
> -
>    hr = DMUS_E_UNSUPPORTED_STREAM;
> -
> - return hr;
>
> }
>
> static const IPersistStreamVtbl persiststream_vtbl = {