On Thu, Apr 12, 2012 at 6:18 PM, Samuel Pitoiset <[email protected]>wrote:
> > > ---------- 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. > I'm sorry for this wrong manipulation. > >> -- >> Anton Khirnov >> > > > > -- > Best regards, > Samuel Pitoiset. > > > > > -- > Best regards, > Samuel Pitoiset. > > -- Best regards, Samuel Pitoiset.
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
