Clemens,
Thanks for your comments.
But Fireworks accepts AM824 labels, and recognizing 0x00 labels is no problem.
Yes. In our experiences, there may be no problem. But I'm concern about the side effect and want to use no labels if possible.
Right now I'm working on merging the playback-only snd-dice driver (without the M-Audio code, which doesn't work) into the kernel, restricted to Weiss devices (they asked for this).
(Setting the pcm_quadlets/midi_quadlets arrays for dual-wire mode is to be moved into snd-dice.)
I think Dice has no SYT-Match mode then the driver need to generate syt for out-stream from the value in in-stream. How do you solve this or Weiss devices don't need this synchronization?
By the way, I did most of implements for this mechanism so hope you to review my patches for it when you have enough time.
I removed S16 because I did not want to duplicate most of the sample writing code, and format conversion should be done in userspace. This would be a regression for users of the snd-firewire-speakers driver (both of them), so I'm not sure if I should keep this.
Do you have a specific reason for wanting S16 support?
I just follow the idea to implement functions which the device has. I have no special reason. But I note that BeBoB supports 20 bit sample, too.
Why? Because many device quirks must be handled in amdtp.c?
Having separate write_samples() implementations would duplicate most of the code in those loops, which I do not think would be better for maintenance purposes.
I assume bad case in which the amdtp.c has many quirks and the effects generate conflict to each driver and many conditions brought longer time and much complexicy to shared code.
I don't understand; how is this related with the payload processing?
I'm sorry. It's not related to MIDI stream...
The amdtp_out_stream_set_pcm_format() is used to set the function to write samples. Here can we add condition to set it referring the value of format in PCM runtime instead ot the function. Then function is automatically selected and each driver don't consider it.
I'm not sure if a separate write_samples() function is appropriate for this; couldn't this be handled by an additional function that is called before or afterwards?
This is better. If I had no idea about this separate write_samples(), I will have applied this way.
These patches are not self-contained, i.e., the driver does not work if only some of them are applied (this happens, e.g., when bisecting).
I realize it. I follow a idea to keep the size of patch small. Next time I'm OK to make them self-contained. Thanks to indicate it.
Despite my criticisms above, if you think that these patches are necessary for your driver(s), I'm not against merging them.
I have no will to do it against your criticisms.
Thanks
Takashi Sakamoto
(Jun 19 2013 02:38), Clemens Ladisch wrote:
Takashi Sakamoto wrote:
Currently snd-firewire-lib handles the contents of data block for AMDTP packet in itself.
But to implement drivers for Fireworks(Echo Audio)/BeBoB(BridgeCo), there is an inconvinience. Fireworks don't use AM824 label for PCM data.
But Fireworks accepts AM824 labels, and recognizing 0x00 labels is no problem.
BeBoB needs to remap PCM and MIDI channels depending on devices.
And Dice(TC Applied Technologies) - I think you understand this than me - needs the mode called as "dual wire". Your snd-firewire-lib in alsa-kprivate.git repository manages to solve these issue.
Right now I'm working on merging the playback-only snd-dice driver (without the M-Audio code, which doesn't work) into the kernel, restricted to Weiss devices (they asked for this).
(Setting the pcm_quadlets/midi_quadlets arrays for dual-wire mode is to be moved into snd-dice.)
But it has restriction about the PCM format (S32 only)
I removed S16 because I did not want to duplicate most of the sample writing code, and format conversion should be done in userspace. This would be a regression for users of the snd-firewire-speakers driver (both of them), so I'm not sure if I should keep this.
Do you have a specific reason for wanting S16 support?
and for me it seems to be hard to maintain.
Why? Because many device quirks must be handled in amdtp.c?
Having separate write_samples() implementations would duplicate most of the code in those loops, which I do not think would be better for maintenance purposes.
Additionally to implement full duplex with synchronization, I need to restart streams for MIDI. Then current amdtp_out_stream_set_pcm_format() is not convinient. I want to add condition for PCM format into each drivers.
I don't understand; how is this related with the payload processing?
Furthermore, 003Rack(Digidesign) - I don't work for this - has more issue. The differences related to payload are:
- the value of syt field in AMDTP packet is always zero
This does not affect the driver.
- The data in each channel is encoded by its own way
The driver needs to refer to and modify the contents of payload.
I'm not sure if a separate write_samples() function is appropriate for this; couldn't this be handled by an additional function that is called before or afterwards?
To solve these device-dependent issues, I want to move the processing of contents in payload from firewire-lib to each devices. This series of patches is for this purpose.
Despite my criticisms above, if you think that these patches are necessary for your driver(s), I'm not against merging them.
sakamocchi (4): firewire-lib: add helper function to set data block size firewire-lib: add callback function for payload firewire-lib: remove unused members and functions firewire-speakers: add functions to handle payload
These patches are not self-contained, i.e., the driver does not work if only some of them are applied (this happens, e.g., when bisecting).
Regards, Clemens