[FFmpeg-devel] udp.c comments/thoughts

[Roger Pack]

On 1/15/14, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Wed, Jan 15, 2014 at 12:59:27PM -0700, Roger Pack wrote:
>> On 1/6/14, Andrey Utkin <me at andrey-utkin.pp.ua> wrote:
>> > 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.
>>
>> The default reported by Ubuntu 12.10 is 212992 which I believe you
>> divide by 2 to get the "real" value, 106496.
>> This means that currently, by default, FFmpeg is actually changing the
>> buffer size to *lower* the default (since it sets it to 64KB).  Which
>> may not be desirable.
>>
>> I could see changing the default to 128KB instead, or possibly adding
>> extra/more logic so that the default is MAX(current, 64KB), thought?
>
> if you send a patch that sets it to MAX(current, 1MB) then ill
> apply it

OK here's a couple followup patchs as well.  One adds a warning if
people don't realize they're not getting what they requested for
buffer_size. FWIW.  The other is docu.

A patch like what you suggested might be even better, of course
(basically, use MAX(default, 64KB).
Thanks.
-roger-
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-udp-cleanup-comments-add-a-warning-if-receive-buffer.patch
Type: application/octet-stream
Size: 1800 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140321/fd714735/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-udp-try-to-clarify-verbiage-on-buffer_size.patch
Type: application/octet-stream
Size: 946 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140321/fd714735/attachment-0001.obj>