On Mon, Aug 13, 2012 at 2:25 PM, Martin Storsjö <mar...@martin.st> wrote:
> On Mon, 13 Aug 2012, Samuel Pitoiset wrote:
>
>> ---
>> libavformat/rtmpproto.c | 128
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 128 insertions(+)
>
>
> Please squash in patch 5 with this one. It does not make any sense to add a
> new option without any code that actually uses it.
Ok.
>
>
>>
>> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
>> index f6e6538..b732fc5 100644
>> --- a/libavformat/rtmpproto.c
>> +++ b/libavformat/rtmpproto.c
>> @@ -41,6 +41,10 @@
>> #include "rtmppkt.h"
>> #include "url.h"
>>
>> +#if CONFIG_ZLIB
>> +#include <zlib.h>
>> +#endif
>> +
>> //#define DEBUG
>>
>> #define APP_MAX_LENGTH 128
>> @@ -800,6 +804,125 @@ static int rtmp_calc_swf_verification(URLContext *s,
>> RTMPContext *rt,
>> return 0;
>> }
>>
>> +#if CONFIG_ZLIB
>> +static int rtmp_uncompress_swfplayer(uint8_t *in_data, int64_t in_size,
>> + uint8_t **out_data, int64_t
>> *out_size)
>> +{
>> + z_stream zs;
>
>
> Please zero-initialize this one, to make sure all other fields that you
> don't touch are properly initialized.
The example of proper use zlib doesn't zero-initialize the stream but
it's probably better.
>
>
>> + void *ptr;
>> + int size;
>> + int ret = 0;
>> +
>> + zs.avail_in = in_size;
>> + zs.next_in = in_data;
>> + ret = inflateInit(&zs);
>> + if (ret != Z_OK)
>> + return ret;
>> +
>> + do {
>> + uint8_t tmp_buf[16384];
>> +
>> + zs.avail_out = sizeof(tmp_buf);
>> + zs.next_out = tmp_buf;
>> + inflate(&zs, Z_NO_FLUSH);
>
>
> If you're decompressing as long as the output buffer is full, you would need
> to use Z_SYNC_FLUSH I think.
Not sure, I followed the basic example of zlib http://www.zlib.net/zpipe.c .
>
>
>> + size = sizeof(tmp_buf) - zs.avail_out;
>> +
>> + if (!(ptr = av_realloc(*out_data, *out_size + size))) {
>> + ret = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + *out_data = ptr;
>> +
>> + memcpy(*out_data + *out_size, tmp_buf, size);
>> + *out_size += size;
>> + } while (zs.avail_out == 0);
>
>
> It's better to check the return value of inflate I think. If it returned
> Z_STREAM_END, break the loop. If it returned Z_OK, continue. If it returned
> anything else, break.
Ok.
>
>
>> +fail:
>> + inflateEnd(&zs);
>> + return ret;
>> +}
>> +#endif
>> +
>> +static int rtmp_calc_swfhash(URLContext *s, RTMPContext *rt)
>> +{
>
>
> You don't need to pass both these pointers, when you can get the RTMPContext
> as s->priv_data here.
Indeed.
>
>
>> + char proto[8], host[256], path[1024], url[2048], swfhash[32];
>> + uint8_t *in_data, *out_data, *swfdata;
>
>
> in_data and out_data are uninitialized here, and you free them in fail.
> Guess what happens if you try to free them before they're initialized?
A wonderful segault... =)
Let's initialized them to NULL then.
>
>
>> + int64_t in_size, out_size;
>> + URLContext *stream;
>> + int port, swfsize;
>> + int ret = 0;
>> +
>> + av_url_split(proto, sizeof(proto), NULL, 0, host, sizeof(host),
>> &port,
>> + path, sizeof(path), rt->swf);
>> +
>> + if (port < 0)
>> + port = 80;
>> +
>> + /* Get the SWF player file. */
>> + ff_url_join(url, sizeof(url), proto, NULL, host, port, "%s", path);
>
>
> Why do you split the url just to reassemble it again without any
> modifications whatsoever? This will break an url that would happen to have
> authentication in it (since you don't parse that field), and is just
> generally useless. The original url given by the user can be used just as
> is. You don't need to set a default port here - if the protocol happened to
> be e.g. http, fetching http://server/file will default to port 80 just fine.
> And the "url" could just as well just be "foo.swf", a local file in the
> current directory.
I see.
>
>
>> + if ((ret = ffurl_open(&stream, url, AVIO_FLAG_READ,
>> + &s->interrupt_callback, NULL)) < 0) {
>> + av_log(s, AV_LOG_ERROR, "Cannot open connection %s.\n", url);
>> + goto fail;
>> + }
>> +
>> + if ((in_size = ffurl_seek(stream, 0, AVSEEK_SIZE)) < 0) {
>> + ret = AVERROR(EIO);
>> + goto fail;
>> + }
>> +
>> + if (!(in_data = av_malloc(in_size))) {
>> + ret = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> +
>> + if ((ret = ffurl_read_complete(stream, in_data, in_size)) < 0)
>> + goto fail;
>> +
>> + if (!memcmp(in_data, "CWS", 3)) {
>> + /* Decompress the SWF player file using Zlib. */
>> + if (!(out_data = av_malloc(8))) {
>> + ret = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + *in_data = 'F'; // magic stuff
>> + memcpy(out_data, in_data, 8);
>> + out_size = 8;
>> +
>> +#if CONFIG_ZLIB
>> + if ((ret = rtmp_uncompress_swfplayer(in_data + 8, in_size - 8,
>> + &out_data, &out_size)) < 0)
>> + goto fail;
>> +#else
>> + av_log(s, AV_LOG_ERROR,
>> + "Zlib is required for decompressing the SWF player
>> file.\n");
>> + ret = AVERROR(EINVAL);
>> + goto fail;
>> +#endif
>> + swfsize = out_size;
>> + swfdata = out_data;
>> + } else {
>> + swfsize = in_size;
>> + swfdata = in_data;
>> + }
>> +
>> + /* Compute the SHA256 hash of the SWF player file. */
>> + if ((ret = ff_rtmp_calc_digest(swfdata, swfsize, 0,
>> + "Genuine Adobe Flash Player 001", 30,
>> + swfhash)) < 0)
>> + goto fail;
>> +
>> + /* Set SWFVerification parameters. */
>> + av_opt_set_int(rt, "rtmp_swfsize", swfsize, 0);
>> + av_opt_set_bin(rt, "rtmp_swfhash", swfhash, 32, 0);
>
>
> You don't need to use the AVOption functions for setting values in your own
> context, you could just as well do this:
I know...
>
> rt->swfsize = swfsize;
> av_free(rt->swfhash);
> rt->swfhash = av_malloc(32);
> if (!rt->swfhash) {
> ret = AVERROR(ENOMEM);
> goto fail;
> }
> memcpy(rt->swfhash, swfhash, 32);
>
> Although this is a bit longer than what you currently have, so perhaps you
> can keep av_opt_set_bin.
... and the current code is shorter. :)
--
Best regards,
Samuel Pitoiset.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
