Opened 3 years ago

Closed 3 years ago

Last modified 12 months ago

#12827 closed Bug Report - General (fixed)

Raspberry Pi: Digital Audio does not work

Reported by: Peter Bennett Owned by: Peter Bennett
Priority: minor Milestone: unknown
Component: Ports - rPi Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description (last modified by Peter Bennett)

On Raspberry Pi when enabling Dolby Digital or DTS with ALSA device, all that is heard is a clicking sound, even when the hdmi audio device is selected. When using OpenMAX:hdmi device, passthru of DTS is allowed, but playback is unacceptably choppy. Both audio and video suffer from interruptions.

Change History (11)

comment:1 Changed 3 years ago by Peter Bennett

Status: newaccepted

comment:2 Changed 3 years ago by Peter Bennett

Version: 0.27.6Master Head

comment:3 Changed 3 years ago by Peter Bennett

Description: modified (diff)

comment:4 Changed 3 years ago by Peter Bennett

Output of Dolby digital passthru audio to a surround sound system works and gives sound, but the audio and video are jerky. This is because the omx audio system seems to give incorrect and inconsistent values for audio latency (i.e. amount of audio in the sound card that has not yet been played). The result is that AVSync reports Video is behind audio (too slow) for a few seconds and drops some frames to fix it, then reports that Video is ahead of audio for a few seconds, and slows video down to compensate. Thus the video is alternately speeded up and slowed down every few seconds and the audio is paused and restarted every few seconds.

The audio latency values are incorrect only in the digital passthru case. If playing digital stereo or analog, it works perfectly.

If I set very high values for MAXDIVERGE and DIVERGELIMIT so that AVSync ignores the errors, the video plays very well. 5.1 channel Dolby Digital audio is good, the surround sound receiver reports it as Dolby Digital. The video is in good sync with the sound. However this is not a solution, it will certainly get out of sync soon.

Options

  1. Try to get some code that will estimate the latency based on amount of data passed in and elapsed time. I am afraid this would also gradually get worse over time. Also I am not sure how to handle pauses.
  1. Try to minimize latency by passing data to audio only when it is needed. This is difficult because audio is in a ring buffer before going to the sound device and the ring buffer does not keep track of time codes.
  1. Assume some fixed value for sound card latency all the time.
  1. Average the sync calculation over 30, 75, 100, or 300 frames. Currently it is averaged over 4 frames. This assumes that on average over time the sound card latencies will be correct. I am afraid this may cause other problems, for example when using softblend and displaying OSD the AVSync has to compensate for the extra CPU load by dropping some frames. That works well but will not work with the averaging calculation.

I am not sure how to proceed.

comment:5 Changed 3 years ago by Peter Bennett

I have my fix ready. I am using option 4 above. Average over 60 frames if OpenMAX Audio with AC-3 passthru is specified.

The code is using setting "AVSyncAveraging" for the averaging number. It defaults to 4 as in existing code, and to 60 when using OMX audio with AC-3 passthru. If you override AVSyncAveraging on the command line it will use the override value for every playback.

I added a setting "PlayerMaxDiverge" to replace the #define MAXDIVERGE 3.0f. I also removed the #define DIVERGELIMIT 30.0f because that was having no effect on processing except limiting the divergence value reported in the log.

The AVSync code should work identically to what it did before when not using OpenMAX with AC-3. Some things that were hardcoded constants are now member variables calculated against the avsync_averaging value and will come to the old hardcoded values when avsync_averaging defaults to 4.

Another change that was necessary was removing the spdif container in the case of OpenMAX passthru. The bitstream is passed to OMX without the spdif container.

I am deleting an unused and untested class AudioDecoderOMX. This supports a hardware audio decoder which cannot be used for licensing reasons and the code is not tested.

http://openelec.tv/forum/124-raspberry-pi/59070-rpi-dts-gpu-decoding-lobby-dts-inc-now

The change is not yet committed to MythTV repo. See the below commits in my repository to review the changes before I commit them to MythTV.

Delete unused and untested class AudioDecoderOMX (see line 764)
https://github.com/bennettpeter/mythtv/commit/d50b3c4be37007782668ca6dfa451626e7ec1612

AVSync changes
https://github.com/bennettpeter/mythtv/commit/06fc4834e7fcf8d597c2307f789c66886d74b11c

comment:6 Changed 3 years ago by Mark Spieth

quick review:

  • may be better to disable omx decoder instead of delete in case future functionality allows it again (if its not you who deals with it). perhaps it decodes other formats than DTS too.
  • the constraints on diverge were intended to prevent limit cycling of the control algo. defensive programming.
  • what is the root cause for the hdmi audio rate issue? possibilities
    • actual rate is not what is specified
    • processing latency of AC3 re-encoder (if used timestretch/nonnative ac3) is too large or processing latency of block is too variable or processing time is too large for pi.
    • other systemic manipulation of the output audio a la pulse (not sure about this in pi)
    • hdmi out is block outputted by the hardware in larger chunks and not divisible by the AC3 frame size (15ms??). this can be tested by determining/observing the absorption rate of the output stream.
  • should the avsync averaging number be pulled from the audio device instance instead of cluttering the avsync/player with audio specific config? common base class value for most.
  • avsyncaveraging is config now, perhaps also for the new openmax hdmi value (inside the openmax config value getter)?
  • why /3 when calculating avsyncinterval instead of /4? I know it works the same when the value is 4 with an interval of 0. curious.
  • diverge will be 0 for the passes where avsync_next indicates it is not set. Is this intended?

All I can think of for now. Looks fine.

comment:7 Changed 3 years ago by Peter Bennett

Thank you for the detailed review. I appreciate your comments. Many of these are issues I have been struggling with over the last few weeks. Here are my thoughts and responses.

The reason I deleted omx decoder is that it was in the same source file as the omx renderer code. Some code was similar between the two classes and it was confusing to me. I was fixing errors in the decoder and wondering why nothing was changed. The decoder was running and putting out error messages, although it was never actually being passed any data. The source is still in github if you look back to the correct version. How would you propose disabling it? I could put it in a separate file that is never compiled. Is that better? I think that could also be confusing for people. Having dead code around also adds work for those who do not know it is dead code and are doing some sort of maintenance, for example updating logging throughout the system if there is a new logging method. Also people looking to fix a problem could waste time looking through the dead code, like I did.

The constraint on diverge (DIVERGELIMIT 30) may have been intended as a sort of governor, but the way it was implemented had no effect other than making logging messages incorrect. If the divergence is more than MAXDIVERGE (3), the action is the same, drop a frame or double the rate, depending on the sign of the divergence. It makes no difference to the logic how much it diverges. The expectation is next time you come into the routine the divergence will be less due to the previous action, until eventually it is less than MAXDIVERGE.

I have been pondering the reasons for the audio rate issue. At one point I thought it was calculating based on frame sizes and should have been using compressed audio rate, since the audio being passed through is compressed. However when I changed to use a calculation based on bitrate, audio sync was way off, and still jumping around from too slow to too fast and back again. We are not using a re-encoder, and pulseaudio is turned off. OpenMAX audio bypasses both ALSA and pulseaudio and seems to talk directly to the hardware. Using the average calculation seems to give a good result, but adjustments to the sync will be more gradual so anything which needs frequent adjustments will not work well.

Pulling the avsynv averaging number from the device class - I thought about this and it seemed it is probably not something many classes would use. If you think that is better I will do it. I have to make sure of the timing of creating of the audio output class to make sure it is available at the time we need to get the numbers.

Are you suggesting I should have separate settings for AVSyncAveraging and OMXHDMIAVSyncAveraging? The setting I am using for AVSyncAveraging as I have created it will override for all audio types. I see it mainly as a testing tool. If end users actually make use of it they probably only need 1 setting. They can set it for their current audio selection, which should not change often.

avsync_interval = avsync_averaging / 3 - 1; The 3 here is to set a reasonable rate for divergence checking. It is based on the max_diverge. The value of 3 should actually be specified as max_diverge. I will change it. When averaging the divergence over 60 frames instead of 4, it takes much longer for a change to be reflected in the divergence, so I need to space out the frequency of checking for divergence. So I only check divergence every 20 frames.

diverge is 0 when avsync_next is not set. This is to force adjustments to occur only every avsync_interval. By having diverge as 0 no adjustment (dropping or speeding frame) is done at that frame. In the default case where averaging is 4, divergence is calculated every time.

comment:8 Changed 3 years ago by Mark Spieth

  • the omxdecoder is only active if its in a list of choices, like audio devices. remove it from said list :-)
  • audio rate Im talking about is the output sample rate. If this cant be measured due to lack of api, then there is no choice. This is what the rest of the audio devices do. By this I mean that the current number of audio samples in the output audio buffer is measured. The absorption rate of this output buffer is what I meant by block processing of this output buffer. This input/source bitrate is not a factor. The input/source frame rate is however useful, and each encoding has a duration for each frame. This can be used as a substitute for the output buffer contents if its all you have but it is more complex. This block processing is where averaging is of benefit.
  • Diverge limit you are correct.
  • I will defer to other opinions as to the dissociation of audio device specific parameters/config to the audio device. IMO it decouples better there esp. from a unit testing perspective (e.g. with a mocked audio device). What is done in each is not critical wrt multiple configs or constants.

comment:9 Changed 3 years ago by Peter Bennett <pbennett@…>

Resolution: fixed
Status: acceptedclosed

In 5ddf67e34abeac606fb9bcaf5c4550894d201226/mythtv:

Fix Raspberry Pi digital audio AC-3 passthrough.

Fixes #12827

comment:10 Changed 3 years ago by Peter Bennett <pbennett@…>

In 851a33ec36efa53fe13fa9825f5c2dc815455ea2/mythtv:

Fix Raspberry Pi digital audio AC-3 passthrough.

Fixes #12827

(cherry picked from commit 5ddf67e34abeac606fb9bcaf5c4550894d201226)

comment:11 Changed 12 months ago by Peter Bennett

Owner: changed from Peter Bennett to Peter Bennett
Note: See TracTickets for help on using tickets.