Ticket #240 (closed bug: fixed)

Opened 13 years ago

Last modified 13 years ago

Adopt ffado/libraw1394 to work on juju

Reported by: dajt Assigned to: arnonym
Priority: major Milestone: FFADO 2.1
Component: Version: FFADO SVN (trunk)
Keywords: Cc:
The device the bug applies to:

Description

In test-isorecv-1.cpp and test-isoxmit-1.cpp the code assumes that errno will only be set if raw1394_set_port() failed. Unfortunately, this is not true with juju. errno may be set even when raw1394_set_port() succeeded. You could argue that this is a bug in libraw1394, but I think it's a bug in both libraw1394 and ffado, and should be fixed in both.

Of course, with it patched, ffado-test-isorecv hangs even though nosy shows plenty of iso packets to receive, but that's a different bug.

Attachments

test-iso___-1.cpp.diff (1.6 kB) - added by stefanr on 11/02/09 16:10:51.
proposed fix -- only compile-tested
dumpiso_mod.cpp.diff (0.8 kB) - added by stefanr on 11/02/09 16:15:52.
same bug, same fix in dumpiso_mod.cpp
dumpiso_mod-fix-second-raw1394_set_port.diff (0.7 kB) - added by stefanr on 11/02/09 16:27:42.
proposed fix for a related bug in dumpiso_mod.cpp: second round of raw1394_new_handle and raw1394_set_port lacked error checks
ffado-cleanup.patch (3.6 kB) - added by adi on 11/23/09 08:19:18.
Patch to remove unspecified behaviour (by Jan Fenlason)

Change History

10/27/09 14:07:48 changed by stefanr

There is no bug in libraw1394::raw1394_set_port.

man errno

11/02/09 13:40:15 changed by arnonym

  • version changed from FFADO 2.0-rc2 (1.999.42) to FFADO SVN (trunk).
  • milestone changed from FFADO 2.0 to FFADO 3.0.

Running 2.0 against juju is neither supported nor advised. There are plans (and a gsoc-project?) of porting ffado to the new stack, but thats for a different release probably named 3.0.

11/02/09 14:24:04 changed by stefanr

This bug can theoretically happen on the old stack as well. If it does not occur there, then that's pure luck.

To test for failure of a library function, always check the return code of the function, not errno. Look at errno only in case of error return code.

This is easy to fix -- no need to set any particular target milestone for this. :-)

11/02/09 15:26:22 changed by stefanr

PS: Fortunately there are no further libraries stacked underneath libraw1394. However, libraw1394 necessarily uses file operations and syscalls. These may change errno as well. Hence, always check libraw1394's return codes before you do anything with errno.

11/02/09 16:10:51 changed by stefanr

  • attachment test-iso___-1.cpp.diff added.

proposed fix -- only compile-tested

11/02/09 16:15:52 changed by stefanr

  • attachment dumpiso_mod.cpp.diff added.

same bug, same fix in dumpiso_mod.cpp

11/02/09 16:27:42 changed by stefanr

  • attachment dumpiso_mod-fix-second-raw1394_set_port.diff added.

proposed fix for a related bug in dumpiso_mod.cpp: second round of raw1394_new_handle and raw1394_set_port lacked error checks

11/09/09 11:51:21 changed by arnonym

  • status changed from new to assigned.
  • priority changed from minor to major.
  • milestone changed from FFADO 3.0 to FFADO 2.1.
  • owner set to arnonym.
  • summary changed from ffado-test-iso* don't work with juju (new firewire stack) to Adopt ffado/libraw1394 to work on juju.

This bug is kind of fixed since r1700. Lets keep the bug open for other things related to adopting ffado/libraw1394 to juju...

11/09/09 12:05:08 changed by arnonym

(In [1701]) As far as I can tell these patches don't break the old stack. So apply them... see #240

11/23/09 08:19:18 changed by adi

  • attachment ffado-cleanup.patch added.

Patch to remove unspecified behaviour (by Jan Fenlason)

11/23/09 08:21:34 changed by adi

I've added the ffado-specific patch from one of the mails to linux-1394-devel and attached it. Please apply ffado-cleanup.patch.

It doesn't break operation on ieee1394 and it's also tested on Juju. I was able to run ffado on Juju for one hour without any xrun (I then ended the test).

With the kernel fixes and libraw1394 in place, this really looks good.

11/23/09 14:05:26 changed by arnonym

(In [1731]) Looks as if this doesn't break using the old stack. see #240

11/23/09 14:07:18 changed by arnonym

Last was one was the ffado-cleanup.patch applied to trunk. I didn't yet test it with the new stack as I didn't yet dive into updating/customizing the kernel on ubuntu...

01/01/10 14:18:26 changed by arnonym

  • status changed from assigned to closed.
  • resolution set to fixed.

To the best of my knowledge ffado should work with kernel 2.6.32 and libraw1394 2.0.5 (not yet released). At least it should work within the limits of the new stack ;-) ...but these are worked on.