Server: Improvements to AMCP Commands specificity to interact with channels

Created on 28 Apr 2020  Â·  11Comments  Â·  Source: CasparCG/server

Description

Having been working with these commands for the past few days, I have been finding that there are a few subtle inconsistencies about the way in which they behave. I did touch on this in #1289 however this is a bit more general for the other commands.

Anyway, what I have been thinking, is with the INFO command you only have to specify a channel, whereas with the PLAY and PAUSE commands you have to specify the layer as well as the channel. This is minor, however it does get annoying occasionally. Especially when it comes to things such as the STOP command - if I can type INFO 1 to get all the info on channel one, surely typing STOP 1 can stop all of the media playback, on all the layers of the channel?

Solution suggestion

My proposed solution, is to unify the behaviour of all of the AMCP commands (Where appropriate), to be able to accept and act upon commands when specified for an entire channel, or just a layer of a channel.
For instance:

  • Pause

    • PAUSE 1-1 would pause layer 1 of channel 1
    • PAUSE 1 would pause all layers of channel 1
  • Stop

    • Stop 1-1 would stop playback of layer 1, channel 1
    • Stop 1 would stop the playback of all layers on channel 1

- And so on for the other commands

Additional information

I have been outputting exclusively to ffmpeg consumers to generate multicast streams, so I apologise if this is already a thing when outputting for other consumer types. If it is, then I will change this to a Bug Report, to ask that this gets fixed for the ffmpeg consumers

Also, random note, passing STOP 1 currently does return #202 Stop OK even though it did nothing.

amcp typenhancement

Most helpful comment

No breaking changes should be made to AMCP, there are too many clients out there.

Totally agree with you. In those cases there will be any breaking changes we need to communicate those out and make things obsolete for a period of time (one year or even more depending on the change) so users have a chance to change and adapt to the changes.

All 11 comments

This does make sense to do, but doing so would be a breaking change so is not something that can be easily slipped in. As currently a STOP 1 is equivalent to STOP 1-0.
But what makes this really inconsistent is that CLEAR 1 does as you want, so some of them behave differently. So perhaps STOP 1 should be considered a bug and fixable...

The command responses don't really state what they actually did, and more state whether it errored or not. You get similar behaviour with PLAY returning immediately, even if the clip will take another 5+ frames before being ready or visible. Its a known limitation #850

Sweet - I fully understand the limitations, and expected tweaking the commands would break things. Regarding STOP 1 really being STOP 1-0 are PLAY 1 and PAUSE 1 really PLAY/PAUSE 1-0?

I have noticed that the command returns are not necessarily in sync with when their instruction is carried out, however I did think that they would return an error of some form if the command is not going to do anything, ie 'PAUSE 1'.

Either way, this maybe something to look at for a version with a number of other breaking changes that may need to be made? Unfortunately, C/C++ are languages that I am yet to look into, so I cannot easily make the changes myself in a PR for potential merging at a later date. May have to look into learning them when I some have time, to potentially start helping out with this project - if that would be of use?

This could also be implemented with an ALL keyword, like "STOP ALL 1" similar to https://github.com/nrkno/tv-automation-casparcg-server/commit/ab768993ebef31be5c1eaaecdbf55fc63ee5d1fe 's CLEAR ALL which clears all the channels at once.

Another option would be to support ranges like STOP 1-0 1-9999, that could also be useful if you had layered videos running. Imagine having a background on 1-10, text on 1-11 and foreground on 1-12 and you could PAUSE 1-10 1-12 then RESUME 1-10 1-12.

Perhaps enable wildcards e.g. STOP 1-*.

Important to note is that this likely should not support any extra parameters.

If you go with @hreinnbeck's ALL proposal I'd say it should be STOP 1 ALL

@hreinnbeck While that does sound nice from a human perspective, looking at it from an automated computer controlled view, using the ALL keyword could be a bit of a fun one when it comes to constructing commands to be sent.
Also, how would that behave with commands such as the INFO command? As it stands, currently INFO 1 returns the xml formatted data on channel 1. So would it suddenly need to be INFO 1 ALL to get the same response, with INFO 1-5 to get the info of channel 1, layer 5?

In the interium, until a full unification of the behaviour of these commands is added with all the breaking changes that would bring, I do think something like a wildcard symbol could work. Now while the usage of * is standard, I was thinking maybe something a bit more unorthodox: As we are limited to 9999 layers, what if a layer number above that defaults to all layers?

That way, when it comes to the control software, they can keep the same command constructing code, as they can keep the layer index value being stored as an integer value, instead of needing to also be able to insert a * character. So PAUSE 1-10000 would effectively become PAUSE 1 pausing all of the layers on channel 1.

In fact, could add in both? That way, when working from the command line to control it, users can use the more familiar and faster * character, while software controllers can use a layer index value of 10000 to simplify the work that needs to be done to implement it in their code - leading to a greater uptake. So both PAUSE 1-* and PAUSE 1-10000 would do the same thing, just that one is quicker for humans working from the command line, and the other is more friendly to implement in code.

Using ALL keeps backwards compatibility and STOP ALL 1 instead of STOP 1 ALL was just so that it can be easily added as a new command without changing the parsers for the original commands.

The ALLkeyword would either refer to all channels or layers (e.g. PAUSE ALL would pause all layers on all channels and PAUSE ALL 1 restricts to channel 1).

No breaking changes should be made to AMCP, there are too many clients out there. The only other option would be to implement versioning upon connect from a client (e.g. SET AMCP3 or something like that). I think that would be good and could free us from a lot of old baggage.

Layers are not limited to 9999 so using 10000 would not work.

No breaking changes should be made to AMCP, there are too many clients out there.

Totally agree with you. In those cases there will be any breaking changes we need to communicate those out and make things obsolete for a period of time (one year or even more depending on the change) so users have a chance to change and adapt to the changes.

I am not to familiar with the parsing code of Caspar server, however from experience writing code to build up commands to be sent, in other projects I have worked on, being able to keep a consistent data type and order in commands does make it easier from a client programmers point of view.
That is why I would personally prefer something like STOP 1 ALL, PLAY 1 ALL, etc, until a new version of AMCP comes out - so long as that would not break to many existing clients?

Layers are not limited to 9999 so using 10000 would not work. >

According to here, there is a maximum number of 9999 layers: https://github.com/CasparCG/help/wiki/Server:-Channels
So using index 10000 should work, unless the total number of layers is not tied to the max index value of a video layer? If so, sorry for my misunderstanding of that.

No breaking changes should be made to AMCP, there are too many clients out there. The only other option would be to implement versioning upon connect from a client (e.g. SET AMCP3 or something like that). I think that would be good and could free us from a lot of old baggage.>

I do think that having the ability to set the command version would be great, allowing for the simpler development and roll out of changes to the AMCP commands.
This could open up the potential of having an AMCP-DEV version of the commands, where formats and syntax could be tested out and feedback provided on, before rolling it out as a usable AMCP-n command version?

Regarding the setting of the AMCP command version to use, there is already an AMCP section in the casparcg.conf file for the server - So maybe an option to set the default AMCP command version to use could be added into there, as well as having a SET AMCP-n command?

I think that 9999 is just from some old PR material, there isn't any limit enforced in code and you can just do PLAY 1-123456 RED.

Versioning would need to be per connection since a CCG server is often used with multiple clients depending on what production they are being used for. I have ~10 year old clients still in use!

Oh ok, thanks for clarifying that about the layers for me. Well, I guess it was a nice idea while it lasted
¯\_(ツ)_/¯

Regarding the command versioning: I was more thinking being able to set the default command version, which still could be overridden per connection if required would be nice just to reduce what is needed to start up a connection.
As it would mean you would not need to run the set command if you are using a custom client written to use just that one format the server is defaulting to using.
So maybe it could be considered a soft default of sorts? It's the default version the server uses, that is lower in priority than the version that may be set at an individual connection level.

Regarding the command versioning: I was more thinking being able to set the default command version, which still could be overridden per connection

That would be perfectly fine.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

petterreinholdtsen picture petterreinholdtsen  Â·  61Comments

TKooijmans picture TKooijmans  Â·  61Comments

Punkley picture Punkley  Â·  24Comments

jesperstarkar picture jesperstarkar  Â·  35Comments

grahamspr86 picture grahamspr86  Â·  26Comments