Re: [PATCH 1/5] firewire: fix use of multiple AV/C devices, allowmultiple FCP listeners

From: Pieter Palmers
Date: Sat Dec 26 2009 - 08:36:05 EST

Stefan Richter wrote:
Pieter Palmers wrote:
Stefan Richter wrote:
Date: Thu, 24 Dec 2009 12:05:58 +0100
From: Clemens Ladisch <cladisch@xxxxxxxxxxxx>

Control of more than one AV/C device at once --- e.g. camcorders, tape
decks, audio devices, TV tuners --- failed or worked only unreliably,
depending on driver implementation. This affected kernelspace and
userspace drivers alike and was caused by firewire-core's inability to
accept multiple registrations of FCP listeners.

The fix allows multiple address handlers to be registered for the FCP
command and response registers. When a request for these registers is
received, all handlers are invoked, and the Firewire response is
generated by the core and not by any handler.
I'm not convinced that this patch makes sense.

Oh yes, it makes a lot of sense because it simply fixes firewire-core
for the status quo.

To remind, the status quo is in the old stack as well as in the new
stack: It is the responsibility of applications (high-level drivers)
- to sort out whether an FCP request or FCP response is destined to
them,
- to serialize their own FCP transactions.
Neither ieee1394 core nor firewire-core cares for any of the above yet,
nor do they serialize FCP transactions to particular nodes or units or
subunits.

With this status quo in mind, the patch does nothing more or less than
bringing firewire-core's FCP related functionality to the same level as
ieee1394 core's. The design of FCP handling in firewire-core and
ieee1394 is the same, only firewire-core's implementation of it was
lacking until now.

OK, I see.


But what you are criticizing is the design, not the implementation:

[...]

IMHO it is impossible to reliably match an FCP response to its request
without relying on ordering. There is no information in the FCP frame to
allow other mechanisms. If two applications send the same AV/C command,
but with slightly different parameters, there is no way to tell what
response is meant for what application.

True. Though it has not been a practical problem yet. Well, it has not
been a practical problem for video yet (camcorders, set top boxes etc.).
I am saying this based on linux1394-devel and linux1394-user traffic
over many years now and based on occasional querying of distribution
bugzillas.

If it is a practical problem for audio, then I have not heard of it
until now. I guess you may want to run several independent processes
which concurrently control the same FCP target, do you? (Streaming
driver == jack backend or ALSA backend, routing driver == mixer panel, ...)

I'm pretty confident that it is a practical problem, but a bit similar to a race condition. In any case, we have different processes for audio and control indeed, and I have seen them interfere. If you start both the audio process and the mixer simultaneously, failure is highly likely as the discovery process uses some commands very frequently.

This is not a 'practical' problem because most people (learn to) leave enough time between starting jackd and starting the mixer. This makes things usable, but I don't really consider this to be a satisfactory solution.


Furthermore, some (most?) of the devices don't support incoming FCP
commands while another one is still being processed. In any case they
don't have to, and can send a 'REJECTED' response to this second
request. Where does this response end up?

Like with other responses, all FCP listeners get this response and match
it with their respective pending requests. As you describe, this
becomes ambiguous if there is more than one pending request to the same
subunit, carrying the same command.

After re-reading section 8 of the AV/C Digital Interface Command Set
General Specification v4 (TA1999026), I think that the only way to
reliably implement this is by moving the FCP and AV/C transaction logic
into the kernel.

I don't see how you can reliably implement the following scenario using
the current API/ABI:

"""
APP1 sends AVC_cmd_1 to device
device responds with INTERIM

APP2 sends AVC_cmd_1 to device
device responds with REJECTED as it doesn't support two AVC_cmd_1's
this response should not end up at APP1, as that has a correct request
pending.

device sends response to INTERIM, which should end up at APP1.
"""

I think this can only be implemented by tracking which AVC commands have
been sent to what device, regardless of the userspace application (or
kernel driver) that sent them, and disallowing any transactions that
result in scenarios such as the above.

You are right about all this, but it does not devalue the discussed fix
patch.

If or when somebody designs a programming interface and implements the
infrastructure with the features that you described, the functionality
which this patch merely fixes can be removed, but not before.

Of course a hard requirement of such an interface is that the FCP
related parts of present libraw1394 v2 API can be layered on top of it.
It is a dated and deficient API, but it is here to stay. (I.e. the
three FCP listening related functions and the generic IEEE 1394 write
transaction functions.) A soft requirement is that present libraw1394
implementations remain functional. (Without having it thought through
yet, I think both requirements can be meat without disproportional effort.)

Until then, the immediately necessary task is to fix the bug that
firewire-core does not allow more than one FCP listener at a time. And
this is what Clemens' patch is doing perfectly.

I see this, it was a misunderstanding on my part. Sorry for that.

Pieter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/