On Mon, Jul 9, 2012 at 4:45 PM, Martin Storsjö <mar...@martin.st> wrote:
> Hi,
>
> When sending a patchset, please send it all as one git send-email
> invocation, or if not, please set the in-reply-to header properly, so that
> the mails get chained together properly. Now the 3 patches in this series
> have ended up as 3 different mail threads, making it much harder to locate
> them.

Okay, no problem.

>
> On Thu, 5 Jul 2012, Samuel Pitoiset wrote:
>
>> diff --git a/libavformat/rtmpenc.c b/libavformat/rtmpenc.c
>>
>> +static int rtmpe_read(URLContext *h, uint8_t *buf, int size)
>> +{
>> +    RTMPEContext *rt = h->priv_data;
>> +    int ret, off = 0;
>> +
>> +    /* try to read one byte in nonblocking mode */
>> +    rt->stream->flags |= AVIO_FLAG_NONBLOCK;
>> +    ret = ffurl_read_complete(rt->stream, buf, 1);
>> +    rt->stream->flags &= ~AVIO_FLAG_NONBLOCK;
>> +
>> +    if (ret == AVERROR(EAGAIN)) {
>> +        if (rt->out_size > 0) {
>> +            /* send a new request when there is no incoming data to
>> handle */
>> +            if ((ret = rtmpe_send_request(h)) < 0)
>> +                return ret;
>> +        }
>> +        if (h->flags & AVIO_FLAG_NONBLOCK)
>> +            return AVERROR(EAGAIN);
>> +    } else if (ret < 0) {
>> +        return ret;
>> +    } else if (ret == 1) {
>> +        off  += ret;
>> +        size -= ret;
>> +    }
>> +
>> +    ret = ffurl_read(rt->stream, buf + off, size);
>> +    if (ret < 0 && ret != AVERROR_EOF)
>> +        return ret;
>
>
> This isn't good - what if there was 1 byte to read, but only one single
> byte? Then you'll block here without returning to the caller.
>
> This is different from the RTMPT case.
>
> Luckily for you, it's also very much simpler. You shouldn't need any of this
> complication. There's no need to buffer up the outgoing requests at all.
> Since RC4 is a stream cipher, you can just encrypt as many or few bytes you
> want to send in rtmpe_write, and send it right away. Then this function also
> becomes much simpler.
>
> The only case where you need to care about nonblocking mode is when
> publishing data. At the start of rtmpe_read, you probably want to do it like
> this:
>
>
> rt->stream->flags &= ~AVIO_FLAG_NONBLOCK;
> rt->stream->flags |= h->flags & AVIO_FLAG_NONBLOCK;
>
> That is, if the caller has set us to nonblocking mode, do the lower level
> read in the same mode. And if not, clear it.

Okay, I'll improve that part of code.

>
> I didn't find anything else that looked immediately wrong right now,
> although I didn't read it that thorhoughly. Looking forward to see the
> version with gnutls/nettle/gcrypt support.

Yes, I'm trying to add the GnuTLS support but it doesn't work yet.
However, event if it's not specified in the commit message, this patch
is still in work in progress state.



-- 
Best regards,
Samuel Pitoiset.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to