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

[Ramiro Polla]

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.

Ramiro