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.
> +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.
> +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.
> + 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
> +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?
> +
> + if ((ret = find_invoked_method(rt, pkt_id, &invoked_method)) < 0)
> + return ret;
Doesn't look right at all, if a method is not tracked and you get a
reply instead of ignoring it you return invalid_data.
--
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
