[vlc-devel] [PATCH] Integrate the RTMPdump library to play rtmp and rtmpe streams.

[Daniel Tisza]

Hi,

Thanks for the comments.
I made the suggested changes (details below).
Attached the new independent patch.
Can be tested with the playlist from previous message.

>> Changed to HAVE_RTMPDUMP_ACCESS_H
>> Changed to HAVE_RTMPDUMP_RTMPDUMP_H
> This time it look too much like the define created by autoconf

Changed to RTMPDUMP_ACCESS_HEADER_H and RTMPDUMP_HEADER_H.

> You could also use PKG_ENABLE_MODULES_VLC()

Changed to use PKG_ENABLE_MODULES_VLC().
This really is much nicer.

>> +libaccess_rtmpdump_plugin_la_SOURCES = \
>> +     rtmpdump.c \
>> +     access.c \
>> +     $(NULL)
>
> You also need to list the .h else they don't get distributed.

Changed to include the .h

>> + * Copyright (C) 2002-2010 the VideoLAN team
>
> Are you really relying on pieces of code written in 2008.

The patch contains basically new files...changed to Copyright (C) 2010.
Now it would reflect the creation/modification date.

>> +#define FLASHVER_LONGTEXT N_( \
>> +    "Version of the Flash plugin used to run the SWF player." \
>> +    "The default is LNX 10,1,53,64" )
>
> Missing space after full-stops

Added spaces after full-stops.

>> +#define HOSTNAME_TEXT N_("Name of the host to connect to")
>> +#define HOSTNAME_LONGTEXT N_( \
>> +    "Name of the host to connect to." )
> No need of an extra string to repeat yourself. So either you expand this
> one or shorten the short description (or use the same define for both)

Shortened the short descriptions and removed repetition.

> VLC already has an option for that (--start-time)

Changed to use VLC options start-time and run-time.

> Generally, a lot of the long help start by repeating the small help
> which is not that good.

Removed repetition.

> A lot of the shorttext could be shorten: "URL of the SWF player" -> "SWF
> player URL" and so on.

Shortened short descriptions.

> I might have read to quickly but i don't see where you parse the MRL.
> Things like hostname and port should be passed in the MRL rather than an
> option. It would be more user-friendly

Changed to parsing the MRL from p_access.
It is still possible to override the parsed value(s) with option(s), if given.
This overriding of individual properties is one of the desirable
features of librtmp.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rtmpdump3.patch
Type: text/x-patch
Size: 25091 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100630/ac1103b3/attachment.bin>