[vlc-devel] [PATCH] prefetch: remove read size, always request maximum

Rémi Denis-Courmont remi at remlab.net
Thu Feb 6 15:02:45 CET 2020


Hi,

I don't follow how that's a protocol rather than implementation issue. And yes, reducing the read size would reopen 21470, 19806 and 19988 - depending on round-trip time. I don't have anything to add to what's already been written on Trac and here.

Le 6 février 2020 15:03:44 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>On Thu, Feb 6, 2020, at 13:47, Rémi Denis-Courmont wrote:
>> Hi,
>> 
>> A plugin should wait for data if it has none available, and return
>whatever data is available up to the requested length.
>
>OK, I guess we should put that on streams.h documentation.
>
>> 
>> Of course if SMB2 behaves as I suspect it does, capping the read size
>will break streaming high rate media on high RTT networks... But it's
>necessary to stream media on low bandwidth networks. That's why I wrote
>that you've already lost if you care about the read size.
>
>This seems to be a limitation of the protocol, not our libs (libsmb2 or
>libsmbclient), cf.
>https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/4801c3e5-dd73-4e1c-b2ba-8ebf73642227
>The client has to send read requests of a fixed size. This make
>impossible to behave as you said.
>
>Still, there is something I don't understand, your commits (the 3.0
>one) say it fix #19988 and #21470, that are SMB bugs. Putting a read
>cap in smb2.c like you said will just make these bugs reappear, no ?
>
>> 
>> Le 6 février 2020 14:36:18 GMT+02:00, Thomas Guillem <thomas at gllm.fr>
>a écrit :
>>> 
>>> On Thu, Feb 6, 2020, at 13:22, Rémi Denis-Courmont wrote:
>>>> Hi,
>>>> 
>>>> I don't think SMB streaming (as opposed to downloading) is a lost
>cause. I do think SMB streaming with libsmbclient implementation is a
>lost cause.
>>>> 
>>>> The prefetch plugin already gives maximum flexibility, within its
>own constraints, to the underlying access to choose how many bytes it
>returns in a given read, as does the core.
>>> 
>>> Then, we have to update the vlc_stream.h pf_read() documentation.
>Nothing says that access modules *should* (and not could) return less
>than the requested size in some case.
>>> 
>>> I still don't know how I should cap the read size for the smb2
>access plugin.
>>> If I cap the smb2 read size using the same old value, it will just
>re-open https://trac.videolan.org/vlc/ticket/19988 and
>https://trac.videolan.org/vlc/ticket/21470 ?
>>> 
>>> Or I missed something ?
>>> 
>>>> 
>>>> It can't get any better without increasing the prefetch buffer size
>(and we have a setting for that already).
>>> 
>>> 
>>> 
>>>> 
>>>> Le 6 février 2020 14:00:55 GMT+02:00, Thomas Guillem
><thomas at gllm.fr> a écrit :
>>>>> I quote you:
>>>>> 
>>>>>  "Badly behaving streams wait for the whole requested amount and
>do not
>>>>>  perform any pipelining/buffering (e.g. CIFS, probably SFTP)."
>>>>> 
>>>>> Why SMB, NFS, SFTP are badly behaving streams ? For me it is just
>like local I/O. You read only what you request.
>>>>> 
>>>>> On Thu, Feb 6, 2020, at 12:28, Rémi Denis-Courmont wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> The access is free to return any non-zero size up to the
>requested read size, so the requested read size can never be too big.
>It can only ever be too small, and that's exactly why this patch made
>sense and still makes sense.
>>>>> 
>>>>> At the VideoLabs office with 1TB network, I don't care, but at
>home with a shitty 2MB wifi, reading chunks by 16MB is terrible.
>Specially when 99% is trashed since the demuxer will only ask few kB
>and seek.
>>>>> 
>>>>>> 
>>>>>> But if you need to worry about the read size, you've already
>lost.
>>>>> 
>>>>> Why am I lost ? Are you saying SMB streaming is a lost cause ? 
>>>>> 
>>>>>> Small read size will cause bandwidth problems and large read size
>will cause latency problems.
>>>>> 
>>>>> Can we find a solution for both cases in the core or the cache
>modules ? I really don't see why I should cap the read size of all
>"badly behaving" streams.
>>>>> If I need to do that, what size should I use ? Should I add one
>more option (likely the same that you removed in prefetch) ?
>>>>> 
>>>>>> 
>>>>>> Le 6 février 2020 12:33:55 GMT+02:00, Thomas Guillem
><thomas at gllm.fr> a écrit :
>>>>>>> Hello,
>>>>>>> 
>>>>>>> This commit and this one on vlc-3.0 :
>https://git.videolan.org/?p=vlc/vlc-3.0.git;a=commit;h=3dfa6d4ddd0922552e6575ded416f62b8ce828f5
>are making the smb2 module ultra slow, around 10 times slower.
>>>>>>> 
>>>>>>> The read size requested by prefetch is really big. This make
>probing / seek ultra slow since a lot  of small read() are executed
>(reproduced with mkv and mp4).
>>>>>>> 
>>>>>>> So, I have one big question: Is the smb2 module supposed to not
>listen to the size argument and always read a smaller size ?
>>>>>>> How can I decide the read size then ? Should this size be
>decided by the core of cache modules instead (like before) ?
>>>>>>> 
>>>>>>> PS:  There is not the same problem with the libdsm module since
>the max read size is caped inside libdsm.
>>>>>>> 
>>>>>>> On Fri, Apr 20, 2018, at 15:45, Thomas Guillem wrote:
>>>>>>> > 
>>>>>>>> On Fri, Apr 20, 2018, at 15:05, Rémi Denis-Courmont wrote:
>>>>>>>>> And I don't really want to hear a commit is dangerous from
>somebody that 
>>>>>>>>> backports stuff as lightly and quickly  as you have lately.
>>>>>>>> That is a bit close to false accusation IMHO.
>>>>>>>> 
>>>>>>>>> This was sent for review, and it was *not* backported.
>>>>>>>> Yes and I was OK with it. It's just lately that I found out
>that this 
>>>>>>>> commit might be dangerous.
>>>>>>>> Dangerous as changing the behavior of some access modules and
>maybe 
>>>>>>>> induce some new bugs (that are access related).
>>>>>>>> 
>>>>>>>>> -- 
>>>>>>>>> Envoyé de mon appareil Android avec Courriel K-9 Mail.
>Veuillez excuser 
>>>>>>>>> ma brièveté.vlc-devel mailing list
>>>>>>>>> To unsubscribe or modify your subscription options:
>>>>>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>>>>>> vlc-devel mailing list
>>>>>>>> To unsubscribe or modify your subscription options:
>>>>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>>>>> vlc-devel mailing list
>>>>>>> To unsubscribe or modify your subscription options:
>>>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>>>> 
>>>>>> -- 
>>>>>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté. 
>>>>>> _______________________________________________
>>>>>> vlc-devel mailing list
>>>>>> To unsubscribe or modify your subscription options:
>>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>>> 
>>>> 
>>>> -- 
>>>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté. 
>>>> _______________________________________________
>>>> vlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>> 
>> 
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté. 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200206/d732b661/attachment.html>


More information about the vlc-devel mailing list