[FFmpeg-devel] RTMPE support

[Kostya]

On Mon, Mar 15, 2010 at 08:48:23AM -0700, Howard Chu wrote:
> Carl Eugen Hoyos wrote:
>> Howard Chu<hyc<at>  highlandsun.com>  writes:
>>
>>> Here's a librtmp wrapper for libavformat. These diffs are needed as well:
>>
>> The preferred method to post such a change is:
>> svn add libavformat/rtmp2.c
>> svn di libavformat>patch.diff
>
> Thanks. This code was not a candidate for inclusion, there are still  
> outstanding problems that need to be resolved first, as I mentioned in my 
> previous message. This is just to get the conversation going again.
>
>> I don't think the patch should disable the current code (did you open
>> bug-reports for the cases where it does not work?) and your new file contains
>> tabs that cannot be committed to svn (please consider running tools/patcheck).
>
>> Before sending a new patch, the usefulness of adding a new external dependency
>> should probably be discussed.
>
> I didn't think a bug report about RTMPE was necessary, it's obvious that 
> the existing code doesn't support it. I agree that this area needs to be  
> discussed; compn raised the question already
>
> http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2009-September/075579.html
>
> but the conversation died. Re: external dependencies - if you want to 
> have RTMPE support, you're going to need to add a crypto library. As I 
> already pointed out in my previous email, if you're going to add a crypto 
> library, you can also get https at the same time.
>
> I mentioned a few problems in the existing code to kostya on 
> #ffmpeg-devel a few hours ago but will summarize more extensively here:
>
> The existing handshake code has a problem, it hangs when trying to 
> connect to an rtmplite server. It also only checks in one place for the 
> handshake digest, while in practice there are two different locations 
> that need to be checked, depending on the server version. It appears to 
> me that this code has not been tested against enough RTMP server 
> implementations.

I agree it has not been tested against many existing RTMP server
implementations for various reasons.

But some of your claims regarding my code are false. For example, it
checks in two places for digest (and has works with plain old
handshake).

> The existing code doesn't implement Pause or Seek. To me, one of the main 
> reasons to even bother with RTMP support inside the player is to have 
> Seek support, otherwise we can just use rtmpdump | mplayer and get linear 
> playback.

Mostly because there's no easy way to hint protocol handler to send
pause. And protocol handler seek takes file offset and not timestamp, so
it's unknown how to translate one into another.

> The existing code only implements the FlashPlayer 9 handshake. Some sites 
> (like Hulu) are now configured to require FlashPlayer 10, and will reject 
> otherwise valid connection attempts that use FP9.

/me does not live in USA so no Hulu here.
But FP10 handshake can be added.

> The existing code doesn't implement RTMPE, doesn't implement RTMPT, and  
> doesn't implement SWF Verification. (Obviously; that's the original point 
> of this Subject.) These features could of course be added to it, but to 
> me that's wasteful duplication of effort.

I agree with that (unless someone wants yet another SSL implementation
in FFmpeg ;).

> The existing code doesn't support audio-only streams. It generates a  
> hard-coded header indicating audio+video content, rather than basing the  
> header on the actual stream content.

Can be fixed. And I've tested it, seemed to work on audio-only streams
fine.

> The existing code doesn't support FMS 3.5 buffer management. FMS 3.5 will 
> stop sending data arbitrarily, independent of the RTMP buffertime that 
> was negotiated. Without being prodded back into resuming, the connection 
> will simply close due to inactivity, leaving you with an incomplete 
> stream.
>
> The existing code doesn't generate BytesRead reports. Some servers will 
> close the connection if these reports are not received at the expected 
> intervals.

Huh? I've added sending "bytes read" report about a month ago.

> The existing code uses a single chunk_size setting, but RTMP chunk size 
> needs two settings, since the server->client and client->server sizes are 
> independent. As it is, when a server sends a larger chunk size to the 
> client (as it always does, when playing media) it will cause the wrong 
> chunk size to be used for subsequent client->server packets.

Can be fixed.

> The existing code doesn't handle several responses that Adobe FMS 
> routinely generates. I'm not sure how critical this is, since I haven't 
> seen this code work yet, but at least it fails to duplicate the behavior 
> of an actual Flash client.

The zeroeth question is should it do so.

> Aside from feature completeness and correctness, I have some other doubts 
> about this code. The ff_amf_write_*() functions don't do any bounds 
> checking; it's possible (however unlikely) for gen_connect() to cause a 
> buffer overrun. There are other basic inefficiencies and such but there's 
> obviously more significant things to worry about first.

It relies on knowledge how much bytes it takes to write data, so buffer
is allocated with knowledge of output data size.

And I admit that I'm lazy and have little interest in RTMP. I try to
maintain it though.

Yet I'd like to hear from major contributors whether we need native RTMP
support or can ditch it for rtmpdump interface. Vad tror du? ?? ???????
? ????? ????????

> -- 
>   -- Howard Chu