[FFmpeg-devel] [PATCH] Yield on AVERROR(EAGAIN).

[Luca Abeni]

Michael Niedermayer wrote:
> On Fri, Mar 05, 2010 at 02:22:18AM +0000, M?ns Rullg?rd wrote:
>> Ramiro Polla <ramiro.polla at gmail.com> writes:
>>
>>> Hi,
>>>
>>> Currently FFmpeg eats 100% cpu on devices that return EAGAIN. Attached
>>> patch makes FFmpeg yield when it gets EAGAIN. In Windows it gets down
>>> to normal cpu levels, but on Linux this didn't work as I expected, it
>>> still eats 100% cpu. Any ideas?
>> It's the wrong approach.  The spec for sched_yield says:
>>
>>   The sched_yield() function shall force the running thread to
>>   relinquish the processor until it again becomes the head of its
>>   thread list.
>>
>> On some kernels this puts the thread out of action for a full tick,
>> which is probably what you are seeing on Windows (yes, it has a
>> different name there).  On a modern Linux system it will merely invoke
>> the scheduler, which will notice that nothing else wants to run and
>> put this thread back on the CPU immediately.  Check with "vmstat 1" if
>> system the call rate shoots up while running this.
>>
>> The correct solution is to make the devices somehow notify when input
>> is available so the process can sleep properly while waiting.  On Unix
>> systems this is typically done with select() on a file descriptor.  On
>> Windows something like WaitForMultiPleObJectS() is used.  Perhaps that
>> is something we could create an abstraction for.
> 
> The question is not only what is the philosophically correct approach but
> also how complex and portable it is.
> And yield() is quite portable and as you see from this patch quite simple.

AFAIK, yield() behaviour is undefined for all the tasks which are not
scheduled using SCHED_RR.
So, using yield() we risk to have code that works with a version on the
kernel but can break in unpredictable ways when the kernel scheduler is
updated.

IMHO, the conceptually better solution would be to use a select() or poll()
like functionality, but a small usleep() should work just fine (and is much
simpler :). If I remember well, ffmpeg.c already uses an usleep() for similar
purposes, no?
So, my proposal would be to just change "sched_yeld()" with "usleep(10000)"
in this patch.



				Luca