On 26.04.2016 6:09, Alex Henrie wrote:
> Cc: Andrew Eikum <aeikum(a)codeweavers.com>
>
> Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com>
> ---
> dlls/qedit/Makefile.in | 3 +-
> dlls/qedit/qedit_private.h | 2 +
> dlls/qedit/tests/timeline.c | 3 -
> dlls/qedit/timeline.c | 10 +-
> dlls/qedit/timelineobj.c | 470 ++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 482 insertions(+), 6 deletions(-)
> create mode 100644 dlls/qedit/timelineobj.c
I think it's better to add this to timeline.c.
>
> diff --git a/dlls/qedit/timeline.c b/dlls/qedit/timeline.c
> index 52123a7..0398f28 100644
> --- a/dlls/qedit/timeline.c
> +++ b/dlls/qedit/timeline.c
> @@ -129,8 +129,14 @@ static HRESULT WINAPI Timeline_IAMTimeline_CreateEmptyNode(IAMTimeline *iface, I
> TIMELINE_MAJOR_TYPE type)
> {
> TimelineImpl *This = impl_from_IAMTimeline(iface);
> - FIXME("(%p)->(%p,%04x): not implemented!\n", This, obj, type);
> - return E_NOTIMPL;
> + HRESULT hr;
> +
> + TRACE("(%p)->(%p,%04x)\n", This, obj, type);
> +
> + hr = AMTimelineObj_create(type, obj);
> + if (FAILED(hr)) return hr;
> +
> + return S_OK;
> }
>
You can simply 'return AMTimelineObj_create...;'.
> +
> +#include <assert.h>
This one is not used.
> +static ULONG WINAPI TimelineObj_inner_Release(IUnknown *iface)
> +{
> + TimelineObjImpl *This = impl_from_IUnknown(iface);
> + ULONG ref = InterlockedDecrement(&This->ref);
> +
> + TRACE("(%p) new ref = %u\n", This, ref);
> +
> + if (ref == 0)
> + {
> + CoTaskMemFree(This);
> + return 0;
> + }
> +
> + return ref;
> +}
Extra 'return 0;' does not add anything.
> +HRESULT AMTimelineObj_create(TIMELINE_MAJOR_TYPE type, IAMTimelineObj **ppv)
> +{
> + TimelineObjImpl* obj = NULL;
No need to initialize it.
> +
> + TRACE("(%d,%p)\n", type, ppv);
> +
> + switch (type)
> + {
> + case TIMELINE_MAJOR_TYPE_COMPOSITE:
> + case TIMELINE_MAJOR_TYPE_TRACK:
> + case TIMELINE_MAJOR_TYPE_SOURCE:
> + case TIMELINE_MAJOR_TYPE_TRANSITION:
> + case TIMELINE_MAJOR_TYPE_EFFECT:
> + case TIMELINE_MAJOR_TYPE_GROUP:
> + break;
> + default:
> + return E_INVALIDARG;
> + }
What happens to returned pointer on type error?
> +
> + obj = CoTaskMemAlloc(sizeof(TimelineObjImpl));
> + if (NULL == obj) {
> + *ppv = NULL;
> + return E_OUTOFMEMORY;
> + }
> + ZeroMemory(obj, sizeof(TimelineObjImpl));
> +
> + obj->ref = 1;
> + obj->IUnknown_inner.lpVtbl = &timelineobj_vtbl;
> + obj->IAMTimelineObj_iface.lpVtbl = &IAMTimelineObj_VTable;
> +
> + *ppv = &obj->IAMTimelineObj_iface;
> + return S_OK;
> +}
>
memset is not useful, you're setting all the fields anyway. Also, why do
you need inner IUnknown for this object?