On Fri, Apr 13, 2012 at 12:22 PM, Martin Storsjö <[email protected]> wrote:

> 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.
>

Yes, I understand now. Thanks for your remarks.


>
>
> 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
>
>


-- 
Best regards,
Samuel Pitoiset.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to