On 07/31/2012 04:54 PM, Samuel Pitoiset wrote:
> On Tue, Jul 31, 2012 at 3:26 PM, Luca Barbato <lu_z...@gentoo.org> wrote:
>> On 07/31/2012 02:34 PM, Samuel Pitoiset wrote:
>>> ---
>>> libavformat/rtmpproto.c | 264
>>> +++++++++++++++++++++++++++---------------------
>>> 1 file changed, 147 insertions(+), 117 deletions(-)
>>
>>> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
>>> index 501e0ed..909c290 100644
>>> --- a/libavformat/rtmpproto.c
>>> +++ b/libavformat/rtmpproto.c
>>> @@ -52,15 +52,17 @@
>>
>>
>>
>>> + int invoke_size; ///< size of invoked methods
>>> buffer
>>> + int invoke_capacity; ///< capacity of invoked
>>> methods buffer
>>
>> Why you need size and capacity? What you name size seems the number of
>> elements stored we use nb_${something} and what is referred as capacity
>> seems an early optimization over reallocation and could be named size.
>
> Ok.
>
>>
>>> +static int ff_amf_read_string(const uint8_t *dst, uint8_t *str, int
>>> strsize)
>>> +static int ff_amf_read_number(const uint8_t *dst, double *val)
>>
>> Those two might clash with what Jordi is working on.
>
> Yes, I know.
>
>>
>>> +static int add_invoked_method(RTMPContext *rt, const char *name, int id)
>>> +{
>>> + void *ptr;
>>> +
>>> + if (rt->invoke_size + 1 > rt->invoke_capacity) {
>>> + rt->invoke_capacity = (rt->invoke_size + 1) * 2;
>>> + ptr = av_realloc(rt->invoke_data,
>>> + rt->invoke_capacity * sizeof(*rt->invoke_data));
>>> + if (!ptr)
>>> + return AVERROR(ENOMEM);
>>> + rt->invoke_data = ptr;
>>> + }
>>> +
>>> + rt->invoke_data[rt->invoke_size].name = strdup(name);
>>
>> I wonder if all the strings we use aren't constant and globally defined.
>> We might spare a strdup and a free.
>
> I'll try to do that.
>
>>
>>> + rt->invoke_data[rt->invoke_size++].id = id;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int find_invoked_method(RTMPContext *rt, int id, char **buf)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < rt->invoke_size; i++) {
>>> + if (rt->invoke_data[i].id != id)
>>> + continue;
>>> +
>>> + *buf = rt->invoke_data[i].name;
>>> + return 0;
>>> + }
>>> +
>>> + return AVERROR_INVALIDDATA;
>>> +}
>>> +
>>> +static void free_invoked_method(RTMPContext *rt)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < rt->invoke_size; i ++)
>>> + av_free(rt->invoke_data[i].name);
>>> + av_free(rt->invoke_data);
>>> +}
>>
>> It frees all of them, the name would be misleading
>
> Indeed.
>
>>
>>> +static int rtmp_send_packet(RTMPContext *rt, RTMPPacket *pkt, int queue)
>>> +{
>>
>> probably "track" is more descriptive.
>>
>>> + int ret;
>>> +
>>> + if (pkt->type == RTMP_PT_INVOKE && queue) {
>>> + char name[128];
>>> +
>>> + if ((ret = ff_amf_read_string(pkt->data, name, sizeof(name))) < 0)
>>> + return ret;
>>> +
>>> + if ((ret = add_invoked_method(rt, name, rt->nb_invokes)) < 0)
>>> + return ret;
>>> + }
>>> +
>>> + ret = ff_rtmp_packet_write(rt->stream, pkt, rt->chunk_size,
>>> + rt->prev_pkt[1]);
>>> + ff_rtmp_packet_destroy(pkt);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> @@ -996,54 +1037,44 @@ static int handle_invoke(URLContext *s, RTMPPacket
>>> *pkt)
>>
>>> + if ((ret = ff_amf_read_number(pkt->data + 10, &pkt_id)) < 0)
>>> + return ret;
>>
>> why 10?
>
> 10 is the length of "\002\000\007_result" (ie. the name of the invoke reply).
It isn't exactly probably using the read+update approach might be
better. It can be done later and not in this patch.
lu
--
Luca Barbato
Gentoo/linux
http://dev.gentoo.org/~lu_zero
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
