[pulseaudio-tickets] [Bug 42804] raop module does not work with shairport

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=42804

--- Comment #47 from Hajime Fujita <crisp.fujita at nifty.com> ---
Hi Matthias,

Thank you for taking a crack on the packet retransmission feature.
I was so excited that I almost forgot to have a dinner. :)

I tried your patch (on v4.0+raop at this moment), but unfortunately it did not
work, in a sense that I still hear some sound jump when a packet retransmission
happens.

After a quick research, I found several issues:
1. "payload type" of the retransmission packet should be 86 (0x56) [1]
2. retransmission packets should be sent to the control port [1], not to the
streaming port
3. based on observation of a packet sequence from iPad to VSX-43, the
retransmission packet seems to have a special structure. shairport's
implementation for retransmission handling [2] supports this observation. 

struct retrans_pkt_hdr {
    uint16_t rtphdr; /* = htons(0x80d6), where 0xd6 = 0x56(payload type) +
0x80(marker == on) */
    uint8_t a; /* unknown; seems always 0x01 */
    uint8_t b; /* unknown; seems some random number around 0x20~0x40  */
    uint16_t orig_rtphdr; /* maybe; seems always htons(0x8060) */
    uint16_t retrans_seq_num; /* in network byte order */
    uint32_t retrans_ts; /* RTP timestamp in the original packet */
};

I almost figured out the structure, but I couldn't figure out meanings in 'a'
and 'b' members above.

I wrote a patch for Matthias's patch (attachment #85150) to try the above idea.
I also included a simple packet loss injection scheme to test the feature
efficiently. It intentionally skips 2 consecutive frames (~= 16ms data) per
each second.
(Of course, this dirty hack should be removed in the final version.)

This works for VSX-43. It is difficult to tell if it works by listening to the
music, but I confirmed by the following method.
1. if I disable the retransmission under packet loss injection, the device asks
for retransmission of the same packet (seq. no.) again and again, and I see a
burst of log output.
2. if I enable the retransmission, retransmission request per packet appears
only once. This proves that the device accepts the retransmission packet.
See send_audio_packet() in raop_client.c to try this.

I'm not sure if this works for other devices, as I'm not 100% sure about the
retransmission packet format.

[1]: http://nto.github.io/AirPlay.html#audio-rtpstreams
[2]: https://github.com/abrasive/shairport/blob/master/hairtunes.c#L476

Besides the functionality issues above, I have a few questions regarding
design.
1. in send_audio_packet(), it seems dangerous to use (retrans_seq_num > 0) as a
flag, as the sequence number may wrap (1 in 65536).
2. I'm not sure if we really need to malloc buffer at resend_packets(). Can't
we just use the buffer stored in the pb? Since malloc may fail, I'd rather
avoid calling malloc whenever possible.

[3]:
http://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/Developer/CodingStyle/

> Btw.: Hajime, you can add another device to your supported device list: Pioneer VSX-922.

Done. Thank you for reporting.

> I think that the volume calculation is wrong.

Thank you for pointing this out. I'll keep this in mind.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pulseaudio-bugs/attachments/20130904/c30a4540/attachment.html>