[FFmpeg-devel] [PATCH 1/3] lavu/error: add AVERROR_REDO.

[Nicolas George]

Le septidi 7 frimaire, an CCXXIV, Marton Balint a écrit :
> I agree that a loop is necessary if a demuxer can return an error with such
> semantics to maintain compatibility.

So basically, the question boils down to what people consider the better
design: adding a loop in every demuxer that needs it, or have the loop in
the framework.

> Okay, there are two different kind of semantics here:
> 
> 1) Packets were discarded becuase the user did not want them, or they were
> empty packets (stuffing). This "error" does not indicate any problem with
> the stream.
> 
> 2) Data was discarded becuase it was garbage. (sync byte lost case)
> This should simply be an AVERROR_INVALIDDATA? Or we should add a new error
> code and pass it simply to the user?

INVALIDDATA would be bad, because it is already used for fatal errors. We
have a big problem: error codes are just integers, and we have no API to
check "this error code is not really an error" or "this error code is fatal
/ non-fatal". Right now, applications check a few known error codes (EOF,
EAGAIN in non-blocking mode) and consider everything else fatal.

This is exactly the same pattern than classic Unix system programming: there
are a few known error codes (EINTR, EAGAIN in non-blocking mode, EINPROGRESS
for non-blocking connect(); EOF is not returned as an error), and everything
else is considered fatal.

Therefore adding new error codes makes the application developer's life
harder.

> Anyway, IMHO the important thing is to define/document that the error code
> you introduce is only used for the first case.

Right now, the application never sees the error code, since the framework
loops on it. If we introduce an option to allow the error to reach the
application, then this option is the place to document it.

> Because EAGAIN is used for both at the moment I guess and neither of these
> is EAGAIN...

I am not sure distinguishing the different cases (packet in a disabled
stream, utility data, corrupted data until a sync word) is very important.

What is bad right now is that EAGAIN means, has always meant, something else
entirely: in non-blocking mode only, data is not available right now but may
be available later. The normal reaction to EAGAIN is to sleep for a little
(or even better: to watch with poll() or similar, but we lack the API). But
if we do that for ignored streams, we end up sleeping when unnecessary, and
that causes reading FLV at ~70 packets per second instead of thousands.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151127/91930b77/attachment.sig>