Message-ID: <4B360E42.80208@joow.be>
Date: Sat, 26 Dec 2009 14:23:14 +0100
From: Pieter Palmers <pieterp@joow.be>
User-Agent: Thunderbird 2.0.0.23 (X11/20090817)
MIME-Version: 1.0
To: Stefan Richter <stefanr@s5r6.in-berlin.de>
CC: linux-kernel@vger.kernel.org, Clemens Ladisch <cladisch@fastmail.net>,
       linux1394-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/5] firewire: fix use of multiple AV/C devices, allow
 multiple FCP listeners
References: <tkrat.edaaa9d0f51812e8@s5r6.in-berlin.de> <tkrat.2eca556d1afddd17@s5r6.in-berlin.de> <4B35CACF.2000302@joow.be> <4B35FB02.1040602@s5r6.in-berlin.de>
In-Reply-To: <4B35FB02.1040602@s5r6.in-berlin.de>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 6086
Lines: 134

Stefan Richter wrote:
> Pieter Palmers wrote:
>> Stefan Richter wrote:
>>> Date: Thu, 24 Dec 2009 12:05:58 +0100
>>> From: Clemens Ladisch <cladisch@fastmail.net>
>>>
>>> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/