a2dp player

[barbibulle]

Simple CLI app to play audio to a speaker or headphones with A2DP.
This includes support for playing SBC, AAC and Ogg/Opus files.

[zxzxwu]

I think these could be a part of OpusMediaCodecInformation?

[barbibulle]

Done.

[zxzxwu]

I think these constants should be flag instead of enum, so that we don't need additional maps and from_discrete_values to build and serialize codec informations (also applicable to SBC and AAC).

class OpusCodecInformation:
  """OPUS codec information."""

  class ChannelMode(enum.IntFlag):
    MONO = 0x01
    STEREO = 0x02
    DUAL_MONO = 0x04


  class FrameSize(enum.IntFlag):
    SIZE_10_MILLISECONDS = 0x08
    SIZE_20_MILLISECONDS = 0x10


  class SamplingRate(enum.IntFlag):
    RATE_48000 = 0x80

  sample_rate: SamplingRate
  channel_mode: ChannelMode
  frame_size: FrameSize

  VENDOR_ID: ClassVar[int] = 0xE0
  CODEC_ID: ClassVar[int] = 0x01

  def __bytes__(self) -> bytes:
    return struct.pack(
        '<IHB',
        self.VENDOR_ID,
        self.CODEC_ID,
        self.sample_rate | self.channel_mode | self.frame_size,
    )

[barbibulle]

Yes, I think that's much better. I've done that for all 3 types now.
Initially, I thought it would be convenient to treat some fields like sampling frequencies as ints instead of enums, but that just complicates everything for no good reason. Using IntFlag for everything is the right approach.

[zxzxwu]

Should we move MediaPacket to a third module so that we can avoid inline imports?

[barbibulle]

Done. MediaPacket is not in rtp.py

[zxzxwu]

This one is left

[zxzxwu]

nit: If there isn't additional works here

Suggested change

	if isinstance(
	codec_capabilities.media_codec_information,
	VendorSpecificMediaCodecInformation,
	):
	if (
	codec_capabilities.media_codec_information.vendor_id
	== vendor_id
	and codec_capabilities.media_codec_information.codec_id
	== codec_id
	):
	has_codec = True
	else:
	has_codec = True
	has_codec = (isinstance(
	codec_capabilities.media_codec_information,
	VendorSpecificMediaCodecInformation,
	) and codec_capabilities.media_codec_information.vendor_id == vendor_id
	and codec_capabilities.media_codec_information.codec_id == codec_id)

[barbibulle]

That's not 100% the same: if we encounter a capabilities for a standard (non-vendor) codec, we want has_codec to be True.

[zxzxwu]

Suggested change

	(granule_position,) = struct.unpack_from("<Q", header, 6)
	(bitstream_serial_number,) = struct.unpack_from("<I", header, 14)
	(page_sequence_number,) = struct.unpack_from("<I", header, 18)
	(crc_checksum,) = struct.unpack_from("<I", header, 22)
	page_segments = header[26]
	(granule_position, bitstream_serial_number, page_sequence_number, crc_checksum, page_segments) = struct.unpack_from("<QIIIB", header, 6)

[barbibulle]

Done

[zxzxwu]

(Just a comment)

From a spec perspective, importing A2DP from AVDTP seems like a reversed dependencies, but top-down parsing is important for usage. I am thinking whether it's possible to hook parsing table from A2DP, but that will require callers to always import A2DP, or we will need to wrap AVDTP and A2DP stubs inside a public module which is too aggresive for now.

[barbibulle]

Yes, it would be nice to have some better way to have a sort of dependency injection here. I haven't found a great way to do that that's not intrusive. I'll try to think of some better way.

[zxzxwu]

This one is left