[FFmpeg-devel] [PATCH 2/2] dshow: handle events in graph

[Don Moir]

> Hi,
>
> On Thu, Dec 6, 2012 at 11:53 AM, Don Moir <donmoir at comcast.net> wrote:
>> Yeah missed that with EAGAIN return value. I was trying to avoid checking
>> the events every loop and wait for the event to be signaled. I tested your
>> code change for performance and it was fine. I think I prefer the following
>> though because its a bit cleaner:
>>
>> /**
>> * Checks media events from DirectShow and returns non-zero on error or EOF.
>> * Also purges all events that might be in the event queue to stop the
>> trigger of
>> * event notification.
>> */
>> static int dshow_check_event_queue(struct dshow_ctx *ctx)
>> {
>>    LONG_PTR p1, p2;
>>    long code;
>>    while (IMediaEvent_GetEvent(ctx->media_event, &code, &p1, &p2, 0) !=
>> E_ABORT) {
>>        if (code == EC_COMPLETE || code == EC_DEVICE_LOST || code ==
>> EC_ERRORABORT)
>>            ctx->eof = 1;
>>        IMediaEvent_FreeEventParams(ctx->media_event, code, p1, p2);
>>    }
>>    return ctx->eof;
>> }
>>
>> ...
>>         if (!pktl) {
>>            if (dshow_check_event_queue(ctx))
>>                break;
>>            else if (s->flags & AVFMT_FLAG_NONBLOCK)
>>                 return AVERROR(EAGAIN);
>>            else
>>                 WaitForMultipleObjects(2, ctx->event, 0, INFINITE);
>>         }
>> ...
>>
>> Also appears some effort is being done to remove unneeded braces.
>
> It is preferable for simple functions not to have any side-effects if
> possible (like setting a field in the context). So just using the
> return value is a better choice for now, while the function is still
> simple.
>
> About the braces, they're already there and there's no harm in leaving
> them. If we remove them now, and we have to add another statement
> later on, we'll have to add them back. This would just clutter the
> diffs. And it should be done in a separate (non-functionality-related)
> commit anyways.

Ok some people are trying to remove braces and some keep adding them... I hope its not a brain freeze if you have to add them later 
at some separate time :) For me personally it just adds to the code clutter which is already prevalent.

Anyway, I believe your patch is ok and hope it gets applied.