---------- Forwarded message ---------- From: Samuel Pitoiset <[email protected]> Date: Thu, Apr 12, 2012 at 6:17 PM Subject: Re: [libav-devel] [PATCH] rtmp: Support 'app', an option which overrides the name of application [GSoC] To: Anton Khirnov <[email protected]>
On Thu, Apr 12, 2012 at 8:00 AM, Anton Khirnov <[email protected]> wrote: > > On Mon, 9 Apr 2012 00:51:26 +0200, Samuel Pitoiset < > [email protected]> wrote: > > Sometimes the URL parser cannot determine the app name automatically, > > so it must be given explicitly using this option (ie. -app). > > --- > > libavformat/rtmpproto.c | 40 +++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 39 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c > > index 7683559..be82f0f 100644 > > --- a/libavformat/rtmpproto.c > > +++ b/libavformat/rtmpproto.c > > @@ -28,6 +28,7 @@ > > #include "libavutil/avstring.h" > > #include "libavutil/intfloat.h" > > #include "libavutil/lfg.h" > > +#include "libavutil/opt.h" > > #include "libavutil/sha.h" > > #include "avformat.h" > > #include "internal.h" > > @@ -41,6 +42,8 @@ > > > > //#define DEBUG > > > > +#define APP_MAX_LENGTH 128 > > + > > /** RTMP protocol handler state */ > > typedef enum { > > STATE_START, ///< client has not done anything yet > > @@ -56,12 +59,13 @@ typedef enum { > > > > /** protocol handler context */ > > typedef struct RTMPContext { > > + const AVClass *class; > > URLContext* stream; ///< TCP stream used in > interactions with RTMP server > > RTMPPacket prev_pkt[2][RTMP_CHANNELS]; ///< packet history used > when reading and sending packets > > int chunk_size; ///< size of the chunks > RTMP packets are divided into > > int is_input; ///< input/output flag > > char playpath[256]; ///< path to filename to > play (with possible "mp4:" prefix) > > - char app[128]; ///< application > > + char *app; ///< name of application > > ClientState state; ///< current state > > int main_channel_id; ///< an additional > channel ID which is used for some invocations > > uint8_t* flv_data; ///< buffer with data for > demuxer > > @@ -822,6 +826,7 @@ static int rtmp_open(URLContext *s, const char *uri, > int flags) > > { > > RTMPContext *rt = s->priv_data; > > char proto[8], hostname[256], path[1024], *fname; > > + char *old_app; > > uint8_t buf[2048]; > > int port; > > int ret; > > @@ -847,6 +852,16 @@ static int rtmp_open(URLContext *s, const char > *uri, int flags) > > > > rt->chunk_size = 128; > > rt->state = STATE_HANDSHAKED; > > + > > + // Keep the application name when it has been defined by the user. > > + old_app = rt->app ? av_strdup(rt->app) : NULL; > > + > > + rt->app = av_malloc(APP_MAX_LENGTH); > > Aren't you leaking rt->app here? > Strings set by AVOptions are malloced, there's no need for strdup I > think. > No, there is no memleaks here. And Martin Storsjö said "IIRC this shouldn't be needed, if you've declared it as an option, the normal option handling logic should free it (regardless if you've set it via an option or set by the protocol code itself)." I have to store the old value of the application name when it has been defined by the user, so strdup() is required. > > + if (!rt->app) { > > + rtmp_close(s); > > + return AVERROR(ENOMEM); > > + } > > + > > //extract "app" part from path > > if (!strncmp(path, "/ondemand/", 10)) { > > fname = path + 10; > > @@ -868,6 +883,13 @@ static int rtmp_open(URLContext *s, const char > *uri, int flags) > > } > > } > > } > > + > > + if (old_app) { > > + // The name of application has been defined by the user, > override it. > > + av_free(rt->app); > > + rt->app = old_app; > > + } > > + > > if (!strchr(fname, ':') && > > (!strcmp(fname + strlen(fname) - 4, ".f4v") || > > !strcmp(fname + strlen(fname) - 4, ".mp4"))) { > > @@ -1013,6 +1035,21 @@ static int rtmp_write(URLContext *s, const > uint8_t *buf, int size) > > return size; > > } > > > > +#define OFFSET(x) offsetof(RTMPContext, x) > > +#define DEC AV_OPT_FLAG_DECODING_PARAM > > + > > +static const AVOption rtmp_options[] = { > > + {"app", "Name of application to connect to on the RTMP server", > OFFSET(app), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC}, > > I believe I already said this on IRC, but let me repeat it here -- > simply 'app' is a bit too short and uninformative for such a relatively > obscure option. Something like 'rtmp_app' would be better IMO. > Indeed, I'm going to replace 'app' by 'rtmp_app". Thanks for the review. > > -- > Anton Khirnov > -- Best regards, Samuel Pitoiset. -- Best regards, Samuel Pitoiset.
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
