[mpd,db] MPD protocol fixes to handling of idle/noidle command and co…

[geneticdrift]

…mmand list.

Command handling:

1. Changed mpd_read_cb() to delegate to mpd_process_line() for each received command line.
2. mpd_process_line() handles idle state and command list state and delegates to mpd_process_command() to handle each command line. If the command was successful it sends OK to the client according the the command list state. Error responses are sent by mpd_process_command().
3. mpd_process_command() parses the args and delegates to the individual command handler.

mpd_input_filter:

1. Removed handling of command lists. They are handled by mpd_process_line().
2. Return BEV_OK if there's at least one complete line of input even if there's more data in the input buffer.

Idle/noidle:

1. Changed mpd_command_idle() to never write OK to the output buffer. Instead it is the responsibility of the caller to decide on the response.

2. Removed mpd_command_noidle() instead it is handled in mpd_process_line(). If the client is not in idle state noidle is ignored (no response sent) If the client is in idle state then it changes idle state to false and sends OK as the response to the idle command.

Command lists:

1. Added command list state to the client context so commands in the list are buffered and only executed after receiving command_list_end.

Connection state:

1. Added is_closing flag in the client context to ignore messages received after freeing the events buffer in intent to close the client connection.

Command arguments parsing:

1. Updated COMMAND_ARGV_MAX to 70 to match current MPD.
2. Changed mpd_pars_range_arg to handle open-ended range.

Command pause:

1. pause is ignored in stopped state instead returning error.

Command move:

1. Changed mpd_command_move() to support moving a range.
2. Added db_queue_move_bypos_range() to support moving a range.

Command password:

1. Password authentication flag set in handler mpd_command_password() instead of in command processor.

Config:

1. Added config value: "max_command_list_size". The maximum allowed size of buffered commands in command list.

[ejurgensen]

Thanks for your contribution. The PR is too large, can you please split it? Start with the fix for the line thing. Also you need to adjust the code style to match the project. E.g. no mid function declarations.

[geneticdrift]

My apologies. I understand why it would have been more convenient to have a step by step smaller commits. But please understand that it's a single ~5000 LOC c file and most of the changes are interdependent. As I'm not an owntone user myself and did not intend to spend too much time on it, I'll leave it as an extended issue report and a proposed fix instead of a pull request in case a future maintainer find it useful.

[ejurgensen]

Completely understandable. In that case thanks a million for taking the time to look into the code and fix these issues. I will see if I can take it from here and do the necessary fixups, so it can be merged.

[ejurgensen]

I had a look at this to see if I could do the remaining work. One question regarding your changes: You removed mpd_command_noidle() and thus deleted this logic:

  if (ctx->events)
    mpd_notify_idle_client(ctx, ctx->events);

Can you explaing why?

[geneticdrift]

Based on MPD source code, noidle doesn't notify pending events, it just leaves idle mode and sends OK.

https://github.com/MusicPlayerDaemon/MPD/blob/9c19368fc789fa7101110f73b6ffae8503ae326d/src/client/Process.cxx#L54C1-L67C1

[ejurgensen]

Ok, I'm weary about removing it though. There must have been a reason @chme added this logic. Maybe this commit explains it, but I can't say I really understand it.

[geneticdrift]

It looks like MPD documentation and source code are in contradiction.

The idle command can be canceled by sending the command noidle (no other commands are allowed). MPD will then leave idle mode and print results immediately; might be empty at this time.

I don't think it's a problem because idle events are asynchronous anyway so clients can't rely on noidle returning changes.

But if you want to add this behavior see the added commit: 0e39b88.

[ejurgensen]

These changes are now in PR #1794 so I am closing this