Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#8847 closed defect (fixed)

Intermittent failure to play a video file in mythtv.

Reported by: James.Dutton@… Owned by: Janne Grunau
Priority: minor Milestone: 0.24
Component: MythTV - Video Playback Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no


In NuppleVideoPlayer?.cpp, there is a line that contains "player_ctx->buffer->Peek(testbuf, testreadsize);" at about line 1196. That function returns how many bytes were read. In most cases the amount of bytes read will = testreadsize. But, I have seen cases where the amount of bytes read is > 0 but less that testreadsize. myth then decides that it cannot read the file. I.e. Assumes is could not read anything and exists. This is wrong behaviour. The file is there, it is just not being sent fast enough to mythfrontend. I think a better approach would be to repeat the Peek call until the returned value is a negative error value implying end of file or similar and otherwise waiting for more bytes to arrive.

Change History (5)

comment:1 Changed 10 years ago by robertm

Status: newassigned

comment:2 Changed 10 years ago by danielk

FYI This is fixed in the patch for #8812.

comment:3 Changed 10 years ago by anonymous

Which bit of patch #8812 fixes this? #8812 is a patch to fix locking issues. This problem is not a locking issue.

comment:4 Changed 10 years ago by danielk

Resolution: fixed
Status: assignedclosed

(In [26101]) Fixes #8812. This primarily fixes a number of locking problems in the RingBuffer? code.

This also introduces 4 optimizations:

1/ If a seek is to a position already contained in the read ahead buffer, we move the read position to that location and do not reset read ahead. 2/ If there is a seek to within 300,000 bytes of the end of the file, read ahead is not reset and instead we enter a special state which makes reads directly. This prevents the read ahead thread from being cleared when in practice this is usually followed by a seek back to the beginning of the thread. 3/ When the read ahead thread has no work to do it now waits for another thread to give it work instead of attempting to read repeatedly. This happens when the buffer is full, the end of file is seen, we've hit an irrecoverable error, etc. 4/ The read ahead read size is now incresed in size by 50% whenever it's not keeping up with the reader instead of increasing in 32K blocks. This results in faster response when the bitrate estimation is bogis (ffmpeg problem) or absent. Shrinkage is still done slowly in 32K blocks.

OpenFile? and subsequently the RingBuffer? ctor and ANN FileTransfer? have also been changed to use a timeout rather than a retry count parameter. OpenFile? was already using a timeout internally so that local and remote files would be treated similarly, but this had not been extended to the interface yet.

MythPlayer?'s Peek code now retries it's peeks for a couple seconds if they fail due to a too small file initially. This has been a problem ever since the LiveTVChain was introduced.. but it became noticable with some ffmpeg changes and then went from being relatively rare to an everytime occurance with the faster RingBuffer?. This fixes #8847.

Further optimations are possible. For instance, opt #2 could just read in the remaining bytes into an aux buffer which would have no effect on local files but shave at least 100 ms of mythstreaming. Opt #2 could reserve a portion of the buffer for skip backs, while right now those are opportunistic. Opt #4 could also resize the read ahead buffer itself, not just the block size; easy to do if we used a vector for the buffer. The bitrate could be passed from the local ringbuffer to the remote one so it could better estimate it's read ahead requirements. We could fix the bitrate calculations in ffmpeg or perform our own.

Note: There are two ANN FileTransfer? calls outside of the C++ code, one in the python bindings and another in the perl bindings. These are were not using the retry param and so should work fine with the change to a timeout. If there were any I missed in my grep that you know of please check them.

Note2: This ups both the binary version and protocol version, please distclean and upgrade all systems.

comment:5 Changed 10 years ago by stuartm

Milestone: unknown0.24
Note: See TracTickets for help on using tickets.