Re: [PATCH v3 2/3] amstream: Implement IDirectDrawMediaStream::SetDirectDraw().

[Nikolay Sivov]

On 6/25/20 9:56 PM, Gijs Vermeulen wrote:
> @@ -1013,6 +1039,10 @@ static ULONG WINAPI ddraw_sample_Release(IDirectDrawStreamSample *iface)
>  TRACE("(%p)->(): new ref = %u\n", iface, ref);
>
> - EnterCriticalSection(&sample->parent->cs);
> - InterlockedDecrement(&sample->parent->sample_refs);
> - LeaveCriticalSection(&sample->parent->cs);
> - if (!ref)
>    {
>        if (sample->surface)
>
> @@ -1131,6 +1161,10 @@ static HRESULT ddrawstreamsample_create(struct ddraw_stream *parent, IDirectDraw
>      object->ref = 1;
>      object->parent = parent;
> - EnterCriticalSection(&parent->cs);
> - InterlockedIncrement(&parent->sample_refs);
> - LeaveCriticalSection(&parent->cs);
> - if (surface)
>    {

I know this was committed, but please take a look, I don't think it's
consistent.
Even if it works for now, it seems better to lock whole CreateSample(),
and not just a fragment that touches this sample counter.

Release part looks wrong, because you probably don't want to decrement
on every Release(), but only when ref == 0.

P.S. setting sample_refs to 0 initially is obviously also redundant.