[rtmpdump] [PATCH] AMF Object Callback

[Howard Chu]

Chris Larsen wrote:
>> So I guess this list has become the C Programming workshop.
>
> Hey, I'm here to learn and to try and help the community. I do work mostly
> in C++ so I'm not a brilliant C coder.

I'm not here to teach newbies, I've got more rewarding ways to spend my time. 
Sign up for a course, go pay for a tutor, whatever. I didn't get involved in 
this project, writing code in the hopes that beginners will learn from it.
I write code that aims to solve my problems, as best as possible. I don't want 
code from newbies coming in either, code that hasn't been fully thought 
through. If you want to play in this sandbox, you need to be able to keep up, 
think for yourself, with no hand-holding. I've already spent more time on this 
email thread than I should.

>> Also, if you want to intercept a particular message and then act on it,
> you probably need to be able to return a result code. I would say
>> 	#define AMF_CB_CONTINUE	0xffff	/* continue with normal processing
> */
>> 	#define AMF_CB_SUCCESS	0	/* callback did everything, stop
> processing */
>> 	/* any other value: error code, stop processing */
>
>> So your prototype should look like:
>> 	typedef int (AMF_ObjectCallback)(RTMP *, AMFObject *, void *);
>> With
>> 	void RTMP_SetAMFCallback(RTMP *r, AMFObject *obj, void *ctx)
>
> Regarding the return code, since the onus is on the client to take any
> actions it needs depending on the object, does libRTMP really care whether
> the client found what it needed to? The client should just return when it's
> done with the object and libRTMP will continue on it's way. A return code
> could be used for logging so I could add that if you want me to. I'll add
> the RTMP and context pointers though.

Obviously you haven't read thru HandleInvoke() very carefully. For various 
messages it immediately triggers a reply. If you're trying to write a callback 
to handle a new authentication secret, or some other interesting keyword, you 
need to be able to slot in somewhere in the processing flow. Ideally you would 
want to just insert the callback behavior inside, before any other replies are 
generated, otherwise you need to duplicate a lot of functionality if you want 
to generate your own reply.

E.g., look at how SecureToken is handled in HandleInvoke. It has to be 
processed before the RTMP_SendCreateStream() call. Or look at SendUsherToken, 
which gets processed *after* RTMP_SendCreateStream(). Basically, if you want 
to be able to write arbitrary callbacks to handle arbitrary new security 
mechanisms down the road, HandleInvoke needs to be completely gutted and 
redesigned to allow that.

>> The other problem with all of this is that it requires the callback to
> duplicate a lot of librtmp's parsing before it can discover if it actually
> needs to do anything.
>
> All of the parsing has been completed before the callback is executed. The
> client just has to include amf.h and it can do whatever is necessary. I
> didn't touch the AMF_Dump calls but I could wrap them in an else statement
> so that if the client has set a callback, the dumps are not performed. Let
> me know if I should do that.
>
>> So again, the question is - what good is this? How do you actually expect
> to be able to use this feature? Give an actual example, one that would
> actually work,
>> given what you've proposed.
>
> The example I have implemented, as has been talked about on the list before,
> is that of authenticating to an ingest FMS server where the CDN has written
> a custom module to authorize publishing.  It's pretty much the same as the
> AMF_Dump function. Some of the CDNs use a multiple connect approach where
> they send an error message with a token and then you have to reconnect using
> that token. When the initial connect fails, my client checks to see if a
> token was received and if so, attempts to reconnect.
>
> void MyClass::AMFCallback(RTMP *r, AMFObject *obj, void *ctx){
>    for (int n = 0; n<  obj->o_num; n++){
>      if (obj->o_props[n].p_type == AMF_STRING&&
> obj->o_props[n].p_name.av_len>  0
>        &&  strncmp(obj->o_props[n].p_name.av_val, "description", 11) == 0
>        &&   obj->o_props[n].p_vu.p_aval.av_val != NULL){
>
>        std::string temp = obj->o_props[n].p_vu.p_aval.av_val;
>        if (temp.find("authentication required notice") != temp.npos){
>            MyClass *client = (MyClass *)ctx;
>            client->auth_value = temp; // parsed of course to get the actual
> value
>        }
>      }
>    }
> }

So in your example you're saving some data into a static global variable, and 
the main app crunches it and adds some resulting info to its next Connect 
request. I guess that works.

Since we're discussing adding a general extension to the library, I would have 
envisioned something where the callback contains all of the logic needed to 
complete a step (such as authenticating to an FMS server). Then the callback 
can be wrapped inside a dynamically loaded plugin, and used (almost) 
transparently by multiple apps. With your approach, every app has to copy your 
code for operating the callback.