[rtmpdump] [PATCH] AMF Object Callback

[Adam Malcontenti-Wilson]

Hi Chris (and others),
I've looked over it and it appears ok, it essentially replicates the
second version of my patch (named librtmp_callback_v6.patch) - I'm not
sure which is better, but I can see that some parts are different. I'm
assuming you wrote this as a modification of my first patch, so you
might not have included some of the changes in the second. If we can
combine both this and my second patch to get one we can all agree on
it would be good.

More comments inline.

On Wed, Aug 3, 2011 at 1:56 AM, Chris Larsen <clarsen at euphoriaaudio.com> wrote:
> Thank you very much for your post Adam, it's a huge help and now I know a bit more about what Howard's after. I modified your first patch with the linked list and such as Howard specified.
>
>> Updated the patch and examples as per suggestions, as well as changing the callback parameter from an AMFObject to a more generic, extendible custom
>> struct with a union, similar to how AMFObjectProperty stores data, so future callback hooks can return something other than an AMFObject.
>
> I don't know if the response needs to be a union since we're only creating a callback for AMF data and not other types, but if ya'll want to use a union that's cool.

The idea is simply because if we wanted to have a callback that
returned something other than AMF, we'd have to make new callback
prototype. By using a RTMPCallbackResponse struct, we can hold
whatever we may need in the future with the same prototype. I think
this is the biggest difference between our two patches.

>> As for comment 2, I disagree here. The code distinguishes between the three states, however the default is that an error in the callback will continue librtmp's
>> processing rather than aborting it, as more often when an error occurs you'd want librtmp to do the sane thing rather than dropping the packet completely.
>> However, for when that's needed I've also included RTMP_CB_ERROR_ABORT return code.
>
> It is nice to have the ability for the callback to return a number of different responses so I implemented Howard's types and added some logging lines to let the user know exactly what the callback did within libRTMP.

I'm not really sure about the difference between the two, sounds more
like a problem on agreeing what a "default" success should actually do
and really just a name change. I'm not sure about doing the check of
the status code again within HandleInvoke and HandleMetadata just for
logging but I guess it's ok.

>
>> For testing, can rtmpsrv be used (or modified) to test auth schemes using callbacks, or is it too much of a 'stub'? Or do we have some list of public rtmp servers
>> using custom crypto to test this on?
>
> Unfortunately I don't have any public servers to use (and I would have included the authentication code directly in HandleInvoke but an NDA is preventing me, hence the desire for a callback). But I did run tests with this patch on Windows and Ubuntu and it worked properly and let me perform the authentication I needed as well as extract status codes for use in my app.
>
> This version also blocks duplicate callback subscriptions. It will also execute all callbacks until one callback tells it to abort. That way you can have a logging callback that watches all messages and then a separate one that takes action on a certain type.

I don't understand what you mean here extactly, wasn't this possible
in my first patch?

>
> Let me know what ya'll think, thanks.
>
> _______________________________________________
> rtmpdump mailing list
> rtmpdump at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/rtmpdump
>
>

Thanks,
adammw111
-- 
Adam Malcontenti-Wilson