xpra icon
Bug tracker and wiki

Opened 11 months ago

Closed 10 months ago

Last modified 4 weeks ago

#1196 closed enhancement (fixed)

merge sound metadata packets

Reported by: Antoine Martin Owned by: alas
Priority: minor Milestone: 1.0
Component: sound Version: trunk
Keywords: Cc:

Description

Noticed with aac+mpeg4 (#1194) where the mpeg4 headers are sent using dedicated buffers, we find 2 metadata packets without timing information, followed by the actual compressed sound data:

sound source emit_buffer data=<type 'str'>, len=80, metadata={'duration': -1, 'timestamp': -1}
sound source emit_buffer data=<type 'str'>, len=8, metadata={'duration': -1, 'timestamp': -1}
sound source emit_buffer data=<type 'str'>, len=269, metadata={'duration': 23219955L, 'timestamp': 3413054693877L}

I don't think we can merge all into a single string, maybe we can at least merge the first two - this is moot anyway since we should bundle them with the sound buffer that follows as an extra attribute.
This can be done between the sound process and the main process unconditionally since they ship together, the client-server connection can use a new capability to decide if the sound packet then needs splitting up.

Change History (5)

comment:1 Changed 11 months ago by Antoine Martin

Status: newassigned

This also affects vorbis+mka, but none the other codecs.
This is very likely to be caused by the muxers matroska and mpeg4.

comment:2 Changed 11 months ago by Antoine Martin

Owner: changed from Antoine Martin to alas
Status: assignednew

Done in r12578 (+minor fixes in r12587 + r12648 + r12650), the CPU savings are substantial, especially since each sound packet gets parsed by the server (or client for mic) and forwarded (re-formatted) to the other end.

The "-d sound" output now looks like this:

new_sound_buffer(source_subprocess_wrapper(17874), 467, \
    {'duration': 23219955, 'timestamp': 3854512471, 'time': 1463140721776}, \
    [80, 8]) suspended=False

(the "packet metadata" is summarized using its length as "[80, 8]")

@afarr: just a FYI, please close together with #1194. This change should be incorporated in the HTML5 client.

Last edited 10 months ago by Antoine Martin (previous) (diff)

comment:3 Changed 10 months ago by alas

Resolution: fixed
Status: newclosed

Finally made my way though #1194.
Closing this one now too.

comment:4 Changed 9 months ago by Antoine Martin

Milestone: 0.181.0

Milestone renamed

comment:5 Changed 4 weeks ago by Antoine Martin

r15175 does the same for the html5 client, and we can mostly get rid of the MIN_START_BUFFERS initial buffering logic added in r14412: we can assume the stream is playable if we have the metadata.

Last edited 4 weeks ago by Antoine Martin (previous) (diff)
Note: See TracTickets for help on using tickets.