Opened 11 years ago

Closed 10 years ago

#9824 closed Patch - Bug Fix (Fixed)

[PATCH] MythPlayer: Fix playback of dvb-s radio channels

Reported by: Lawrence Rust <lvr@…> Owned by:
Priority: minor Milestone: unknown
Component: MythTV - Video Playback Version: 0.24-fixes
Severity: medium Keywords:
Cc: Ticket locked: no


Playback of DVB-S radio channels is broken in several ways:

  • Switching to a radio channel can take considerable time and often results in failure.
  • After the switch the screen often shows a part faded OSD info or a green background.
  • If the channel contains interactive TV (e.g. BBC radio) then OSD elements can't be seen so OSD info for channel change elements are hidden
  • The data rate of some channels is low (64kb) but RingBuffer::CalcReadAheadThresh? doesn't handle these low rates which results in audio underruns.

These patches

  1. Fix the channel changing & OSD issues.
  2. Fix the RingBuffer? thresholds so that audio underruns are avoided.

Attachments (2)

0040-MythPlayer-Improvements-to-DVB-S-radio-playback.patch (13.6 KB) - added by Lawrence Rust <lvr@…> 11 years ago.
0042-RingBuffer-CalcReadAheadThresh-mods-for-low-bit-rate.patch (4.7 KB) - added by Lawrence Rust <lvr@…> 11 years ago.

Download all attachments as: .zip

Change History (24)

Changed 11 years ago by Lawrence Rust <lvr@…>

Changed 11 years ago by Lawrence Rust <lvr@…>

comment:1 Changed 11 years ago by markk

Lawrence - I've just started looking at these patches.

As a general observation (and this applies to various patches you've submitted, not just these), your patches are difficult to review because they contain various unnecessary and largely cosmetic changes that obscure what you are really trying to achieve. For example, converting locks to use QMutexLocker is fine but put them in a different patch.

Given the potential impact of some of these changes, it would also be much appreciated if you explained what you are doing with various changes.

I'd suggest you try and cut them down to what is absolutely needed.

comment:2 Changed 11 years ago by Github

MythPlayer?: Make killdecoder volatile.

Refs #9824

Branch: master Changeset: d50eab53a64e4f76b0cffd458fc645be1f429853

comment:3 Changed 11 years ago by markk

ref 7d78b55f2

comment:4 Changed 11 years ago by Lawrence Rust <lvr@…>

As a general observation (and this applies to various patches you've submitted, not just these), your patches are difficult to review because they contain various unnecessary and largely cosmetic changes that obscure what you are really trying to achieve. For example, converting locks to use QMutexLocker is fine but put them in a different patch.

I don't accept that. All the changes contained in these 2 patches are necessary with the exception of the additional log output which is 'cosmetic'. Your example, mutex lockers, are required here to facilitate returns without explicitly calling unlock. They improve readability and enable additional return points.

These, and my 'various patches' that I've submitted are what I use on my live systems here. I submit them because I have found them useful and believe others may benefit. I use a condensed version of my git log to create patches for others to download and submit the same patches here. Each patch is intended to stand by itself and so sometimes must be complex as here.

I would comment the code and changes more but I notice a distinct lack of such in the existing code base so I have kept with tradition. If you look at my own projects on you will see a much greater level of annotation.

Perhaps these notes will help:

MythPlayer::DisplayNormalFrame? Only call videoOutput->StartDisplayingFrame?() if there's valid video to display otherwise when tuning to a radio station you just see a green matte screen.

MythPlayer::PreProcessNormalFrame? - commented in the patch

MythPlayer::VideoLoop? - calls DisplayNormalFrame?(!noVideoTracks) i.e. check_prebuffer iff there's video.

MythPlayer::JumpToProgram? - add some error logging and remove a time wasting call to ChangeSpeed?. This sppeds up LiveTV channel changing too.

MythPlayer::EventLoop? - refactor some code for performance and to avoid a potential NULL deref

MythPlayer::PauseDecoder? and MythPlayer::UnpauseDecoder? are re-written to prevent deadlock if pausing audio only streams.

MythPlayer::DecoderPauseCheck? changed to match PauseDecoder?

MythPlayer::DecoderStart? - reduce # retries in case the decoder thread is stuck in AvFormatDecoder::GetFrame?

TV::ChangeChannel? - move PauseAudioUntilBuffered?(ctx) after the video is started. This reduces noise and clicks on restarting radio streams.

RingBuffer::CalcReadAheadThresh? - adjust thresholds for slow streams.

RingBuffer::run - handle situation where readblocksize < CHUNk i.e. slow streams

RingBuffer::WaitForAvail? - this is the most dangerous change as it allows read to return a smaller than demanded size. This is necessary for audio only to prevent audio buffer underruns.

These changes should be tested on 64 kbps audio streams e.g. radio caroline on astra 2d.


comment:5 Changed 11 years ago by Github

RingBuffer?: Set the minimum raw bitrate to 64Kb.

A single audio track could be as low as 64Kb (or lower?) and MHEG only channels (i.e. off air) are usually reported as 0Kb (which is probably not far from the truth).

Ref #9824

Branch: master Changeset: a1022c6f508c326026ba5101e5a41fd764651659

comment:6 Changed 11 years ago by Github

OSD: Don't clear the interactive screen when the OSD is shown.

This may not be optimal (i.e. the screen may get a little busy) but I'll see how it goes first. The alternative is to start adding some state management to ensure the interactive screen is redrawn once the menu/info has been cleared.

Ref #9824

Branch: master Changeset: 3e10bc7e7194d83e94412f309ef2420c106e641f

comment:7 Changed 11 years ago by Github

DVB Radio: Fix deinterlacing of dummy video streams.

Dummy frames are generated as progressive but when copied to the player's video buffers the interlacing flags were not being copied across and we were unncecessarily processing them.

Ref #9824

Branch: master Changeset: e763b85d72d094928eb0f12fbd80771eb552ebb7

comment:8 Changed 11 years ago by Github

MythPlayer?: Add a GetFreeVideoFrames? method.

This will be used by AvFormateDecoder::GenerateDummyVideoFrame? to assess how many dummy video buffers are needed.

Refs #9824

Branch: master Changeset: 9b39022943e41bba002bc90e686598dfc1100159

comment:9 Changed 11 years ago by Github

DVB Radio: Refactor generation of dummy video frames.

  • Fill the video buffers completely on each call instead of creating one

frame for each call to AvFormatDecoder::GetFrame?. Audio or MHEG only streams may have very low bitrates and the player will otherwise be starved of buffers.

  • Don't create a dummy frame, just ask the player to clear the returned

frame. This will allow the VideoOutput? method to clear frames in GPU memory OR just ignore the dummy frame when it is not required for display (e.g. VDPAU, VAAPI, OpenGL, DXVA2).

Ref #9824

Branch: master Changeset: 15d65ae2c41d44f037d1b934d0e6750939f22bb3

comment:10 Changed 11 years ago by Github

VideoFrame?: Add a dummy flag.

This will be used by certain video output methods to skip uploading and rendering of the dummy video frame.

Ref #9824

Branch: master Changeset: 5d6c20ff2e6d15f442694b9352c1503aba10718a

comment:11 Changed 11 years ago by Github

MHEG: Produce dummy frames even if there is no MHEG support.

Should fix a few issues for users without MHEG support compiled who tune to an off air channel (DVB-S/T).

Refs #9824

Branch: master Changeset: aa44cb6175e612e6503f8777e0ed30ab8aaabf0e

comment:12 Changed 11 years ago by beirdo

Owner: set to markk
Status: newassigned

comment:13 Changed 11 years ago by markk

Milestone: unknown0.25
Status: assignedaccepted

comment:14 Changed 11 years ago by Github

RingBuffer?: Low bitrate optimisations.

This adds a special case operation for low bitrate streams (e.g. dvb radio and MHEG/interactive only) where the existing fill requirements were high enough that, for live tv and in-progress recordings, the buffer would not fill in time and playaback could and would fail.

I've tried to ensure this is triggered only as needed but there is still the possibility that faulty bitrate detection may enable it (though even in that case there should, fingers crossed, be no adverse effects).

There are still at least 2 other problems playing these very low. Initial stream scanning of MHEG only streams can take over 20 seconds - as the initial probe still requires 32Kb of data - and stopping playback is problematic as the decoder thread is not exiting correctly.

Refs #9824

Branch: master Changeset: d207a0bddddac4cb015746e2a4b4e5f841897c8c

comment:15 Changed 11 years ago by Github

MythPlayer?: Pause the decoder before stopping it.

This ensures the decoder exits cleanly. Vastly improves channel change time when switching away from a radio or interactive only channel (or between radio channels) or speeds up exiting playback.

Refs #9824

Branch: master Changeset: e78269ac292429bb494ba3537a47a8061f58a9f3

comment:16 Changed 11 years ago by Github

Playback: Improved triggers for low bitrate optimisations.

The optimisations added in d207a0bddddac are being triggered unnecessarily. This is due to a combination of different initial thresholds in that commit and lowering the acceptable bitrate in a1022c6f508c3.

The fix is twofold:-

  • adjust the initial bitrate thresholds to ensure any optimisations are

only triggerred for streams with a bitrate below 500Kb/s.

forced to 500Kb/s. This is an extension of the previous hack for H.264 streams.

Overall, this ensures that the previous behaviour (initial fill size of 32KB, rising as needed) is preserved for anything with a video stream and generally in the vast majority of cases.

Fixes recent playback of avi files and, amongst others, BB HD (both detected as zero bitrate).

Refs #9824.

Branch: master Changeset: f6d8c69a9652e5c47618253fe768844211396e6a

comment:17 Changed 11 years ago by markk

Ref e076469a9a0700109182

comment:18 Changed 11 years ago by markk

Owner: markk deleted
Status: acceptedassigned

comment:19 Changed 10 years ago by beirdo

Component: MythTV - GeneralMythTV - Video Playback
Milestone: 0.25unknown
Status: assignednew

Has the reported problem been sufficiently solved with these many commits referring to it?

comment:20 Changed 10 years ago by beirdo

Status: newinfoneeded_new

comment:21 Changed 10 years ago by Lawrence Rust <lvr@…>

Mark has significantly improved performance with low bit rate and radio streams so the problems originally described are largely fixed. There are still a few remaining issues with high latency (http) streams and given that my original patches are now very out of date, it may be best to close this ticket and I'll re-submit updated patches with a description of the remaining issues.

comment:22 Changed 10 years ago by sphery

Resolution: Fixed
Status: infoneeded_newclosed

Closing at reporter's request. Remaining issues will be handled in another ticket.

Note: See TracTickets for help on using tickets.