[vlc-devel] [PATCH] Implementation of Dozer streaming protocol: rework 1

Daniele Lacamera root at danielinux.net
Sat Oct 15 15:17:48 CEST 2016


This second set of patches fixes the issues pointed out in your reviews.

FEC routines have been simplified a lot thanks to your comments, and the
whole procedure of packet recovery is hopefully much more readable and
easier to follow.

This new implementation has been tested with and without packet loss
between sender and receiver, and gave stable results.

There are a few points that need more explanation, or could not be
addressed:

Q1:
Filip Roséen wrote:
> Use helpers available within VLC
> ----------------------------------------------------------------------
>
> One suitable example would be using `vlc_UrlParse` (and relevant
> functions), instead of reinventing the wheel to parse things manually,
> in `modules/access/dozer.c:Open`.
Jean-Baptiste Kempf wrote:
> Can't you use an already existing parser from VLC?

A1:
The parser used in the UDP module has a slightly different logic than
the one implemented in vlc_UrlParse, in particular regarding the
possibility to join multicast flows via '@'. I was not able to find
enough documentation on this regard, but I will be glad to switch
towards a library function for the url parsing as soon as this is done
in UDP module as well.


Q2:
Jean-Baptiste Kempf wrote:
> Do you really need all this in the sys structure?
A2:
The structure contains all the flow-specific variables needed to perform
the recovery.

Q3:
Jean-Baptiste Kempf wrote:
> Do you need all those headers?
A3:
Apparently yes, as removing any of them will result in compile errors.

Q4:
Jean-Baptiste Kempf wrote:
> Why LIBPTHREAD?

A4:
Both modules use threads to read/write from/to streams. Attempting to
link without LIBPTHREAD results in a linker error. Note that the same
happens with the UDP module.

Q5:
Wilawar wrote:
> Windows doesn’t have fd-s (I don’t know how it’s handled inside VLC).
> HANDLE is commonly given as void*.
> Neither 'semaphore' nor 'thread' tell much about the use of those
> variables.
> The type 'int' for the timeout is due to the UNIX time legacy, I
> guess.
> The name 'fifo' is, not explained and does’t give off much
> information.
A5:
Using generic names for fifo/semaphore/thread seems to be very common
among modules sharing the same design (one extra thread with one fifo
and one semaphore). In particular, this structure has been directly
inherited from the udp modules. Regarding windows compatibility, I have
no way to test, but the fd-based communication has been copied 1:1 from
udp, so I suppose that if the udp module works on windows platforms, the
dozer will do as well.

Please note that quite some remarks coming from the reviews address
problems that are also present in the current udp modules. This rework
fixes most of them in the dozer modules. I will be posting another patch
fixing the issues in the udp modules soon.

Thanks again for the time you are willing to spend on this, and if you'd
want to consider for inclusion.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Added-Dozer-access_output-module.patch
Type: text/x-patch
Size: 26865 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161015/028efddf/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Added-Dozer-access-module.patch
Type: text/x-patch
Size: 25235 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161015/028efddf/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-NEWS-Announce-new-dozer-streaming-modules.patch
Type: text/x-patch
Size: 678 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161015/028efddf/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161015/028efddf/attachment.sig>


More information about the vlc-devel mailing list