[FFmpeg-devel] udp.c comments/thoughts

[Andrey Utkin]

On 06.01.2014 21:37, Roger Pack wrote:
> On 1/6/14, Roger Pack <rogerdpack2 at gmail.com> wrote:
>> Hello.  I noticed that in udp.c sometimes (with pthread cancel
>> enabled) it says "Part of datagram lost due to insufficient buffer
>> size" (I think what it meant to say is due to insufficient read
>> request size?)

No principal difference. But we could somewhat improve log message with 
user-friendly advice, if possible. I haven't check deeply this at the 
moment.

>> Also it's quite confusing that fifo_size is in multiples of 188.
>> Unless you an MPEG engineer, nobody anticipates this.  I might suggest
>> adding another setting of "fifo_size_bytes" or something so that
>> people realize there's a difference.

Yep, confusing. Historical issue.
"fifo_size_bytes" sounds a sort of humiliating for this proper option, 
comparing to ugly "fifo_size" option taking multiples of 188. Maybe some 
more "respectful" name could be found?

>> Also I've noticed that the default RECV buffer size is 64K by default
>> for udp packets.  I might suggest increasing this substantially, as
>> this is far too little for many streams, and the OS can typically
>> handle far higher (1M-10MB at least).

Are there any official documents regulating allowed UDP packet sizes? Or 
you tell us what happens to work with current Linux? Maximum which i 
have seen in nature was 0xffff bytes via IPv4 loopback interface.

> Also as a follow-up, perhaps overrun_nonfatal should be default, since
> it's basically just dropping packets, which UDP by default can do and
> does anyway (so currently it behaves slightly differently with pthread
> cancel enabled than otherwise, which is somewhat confusing).

Makes sense, but the default state of this option was chosen in a way 
not breaking previous behaviour.
I am ashamed now that i've named this option "overrun_nonfatal", the 
option should be without negation - e.g. "overrun_fatal", with inverted 
values.

> Also is av_fifo_peek stable?  Might it not accidentally return you a
> memory address that allows you to read past the end of the circular
> buffer, if you read too far?

av_fifo_peek2 seems ugly to me. Peek capability requires another 
interface to be usable. At last such interface is not friendly.

-- 
Andrey Utkin