On Fri, 13 Apr 2012, Samuel Pitoiset wrote:



On Thu, Apr 12, 2012 at 10:26 PM, Martin Storsjö <[email protected]> wrote:
      On Thu, 12 Apr 2012, Samuel Pitoiset wrote:

            @@ -847,6 +852,19 @@ 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.
            +    if (rt->app) {
            +        old_app = av_strdup(rt->app);
            +        av_free(rt->app);
            +    }


Ok, almost there. This won't leak - but, if rt->app is allocated by
av_malloc/av_strdup, and we want old_app to be allocated by that too,
why copy and free the old one, why not just do old_app = rt->app;?
That will do exactly what you need...


Actually, we need to have the app in the RTMP URL in order to extract the
playpath if it hasn't been defined by the user...

So, if we don't copy rt->app when it is not NULL, it should be override when
we extract the playpath part...

This part of code is quite strange in my opinion, and I think I'll rewrite
that later.

Currenty, that patch only overrides the application name when it has been
defined, so that is exactly what you need, right ?

Yes, but you're missing the point.

Currently, you're doing this:

if (rt->app) {
    old_app = av_strdup(rt->app);
    av_free(rt->app);
}
rt->app = av_mallocz(...);

I'm not saying that you should skip moving/copying it into old_app, that's ok. But if you replace the snippet with this instead, it will work just as fine:

old_app = rt->app;
rt->app = av_mallocz(...);

If you don't understand why this works, please take some time to think it through.


Also, if you find code that looks strange, it might be, but don't rewrite it until you _fully_ understand what it currently does.

// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to