[bole5]

03.05.2011 UPDATE: The audio patch is now in the trunk. Please use Keith's official nightly builds.

I am running the latest nightly build.
The playback of certain video files is too fast. This makes watching the video unpleasant. As the video and audio do stay in sync, the audio is speeded up as well and gets distorted. (With noticable 'scratches' in the sound)
It's important to note that running the latest official build I could play the exact same files fine.

Since I too like lukenukem experienced the same issue with a lot of "Discontinuity" errors in my log, I compiled my own build and then went back to the old atv2 branch on https://github.com/xbmc/atv2/commits/ and reverted back the IOSAudioRenderer.cpp until the sound started working. I pinpointed the problem to changes made to the AddPackets() method after the following commit: https://github.com/xbmc/atv2/commit/b0b7...0c39c344b0

If you add the check that makes sure there is free space in the audio buffer before accepting new packets, the problem goes away. In the code that can be done by adding the following code back to the AddPackets():

Code:

if(len > free)
   return 0;

I hope this helps speed up the development. Big thanks to Scott, Zeljko and others who contributed to this great port of XBMC and to Keith for facilitating the "early" testing of the trunk branch.

[lukenukem]

Thanks for trying to squash the problem.

After installing your modified version the problem persists in a different form.
Now, after maybe 10-15 seconds of playback, the audio drops out and the video becomes incredibly slow. I can see the individual frames passing by.

I ran across this problem when playing the same video file as in my previous post.

Here's the corresponding debug log:
http://pastebin.com/w9iM0sM3

[bole5]

After installing your modified version the problem persists in a different form.
Now, after maybe 10-15 seconds of playback, the audio drops out and the video becomes incredibly slow. I can see the individual frames passing by.

Thank you for the debug log. I downloaded the .avi you provided and was able to watch it without any problems with sound or video syncing. I was also able to skip forward and back many times without having any problems. My guess is that now you might be having issues with your samba share. To take the samba out of the equation please try the following:

1. Copy your .avi locally to your atv2. (I created a folder Downloads {/private/var/mobile/Downloads} and added it as a video source)
2. Try to watch this video using my version, and the original nightly version.

My version works fine for me, while the nightly doesn't...

[lukenukem]

Thank you for your kind response. I appreciate you taking the time to download and test the file for yourself.

I've tried transfering the file and playing it locally, but unfortunately the problem persists.
Just to summarize, I can play the same file without any problems on the latest official build.
It would seem to me that a problem like failing samba would have presented itself on the latest official build as well, and that isn't the case.

[davilla]

@bole5, please try this patch ->http://dl.dropbox.com/u/14341410/ios-audio.patch

[bole5]

@bole5, please try this patch ->http://dl.dropbox.com/u/14341410/ios-audio.patch

Thank you for the patch. It didn't fix the problem in my case, but I can see the improvements in stability. In my case I have a video with mono sound which needs to be remapped to 2 channels. Since in your patch you apply the if (len > free) condition only in simple case, I still have problems with sound getting out of sync.

I also noticed that the locks are not released consistently, which I think might be causing the ATV2 to restart/freeze randomly. Do we really need locks for each access of m_dllAvUtil->av_fifo_*?. If so, then why not only for read/write/free actions, but not for size since the av_fifo_size should be thread safe anyway.

Please find the patch that fixes problem for me on pastebin: http://pastebin.com/xEw5Vq3A
The biggest difference is in the method AddPackets(), but I also removed (IMO) unneeded locks and added call to lock.Leave() in Flush(). This version works stable for me after initial testing. I need to test it further to confirm that all my other videos also work correctly...

Thank you again for the great effort and the time spent on this port.

[davilla]

Thank you for the patch. It didn't fix the problem in my case, but I can see the improvements in stability. In my case I have a video with mono sound which needs to be remapped to 2 channels. Since in your patch you apply the if (len > free) condition only in simple case, I still have problems with sound getting out of sync.

I also noticed that the locks are not released consistently, which I think might be causing the ATV2 to restart/freeze randomly. Do we really need locks for each access of m_dllAvUtil->av_fifo_*?. If so, then why not only for read/write/free actions, but not for size since the av_fifo_size should be thread safe anyway.

Please find the patch that fixes problem for me on pastebin: http://pastebin.com/xEw5Vq3A
The biggest difference is in the method AddPackets(), but I also removed (IMO) unneeded locks and added call to lock.Leave() in Flush(). This version works stable for me after initial testing. I need to test it further to confirm that all my other videos also work correctly...

Thank you again for the great effort and the time spent on this port.

I'd like to focus on your needing "if (len > free)" in all cases. In the simple case, len is indeed the number of bytes going into m_Buffer. In the more complicated case, you could be re-mapping to more or less channels so you have to use the number of bytes that would result from the remap as that's what's going into m_Buffer. My patch 'should' handle this but since you still see it, there's a hole somewhere and I'd like to understand this better.

I'd like a sample of your problem video with mono into two-channels so I can reproduce this.

About locks, av_fifo is not thread safe as does not use locks or is lockless in nature. "CSingleLock lock(m_critSection);" locks the section on creation, unlocks on delete. Where you see me doing "lock.Leave();", I want to unlock early, before the lock var is auto-deleted when the routine returns. Reading av_fifo_size ' without a lock should be ok, worse case is you read that the size is less than indicated because OnRender drained it.

There is one area that I don't like using a lock, and that's in RenderCallback which calls OnRender. CoreAudio says you must not block here as the thread that calls this is running with a higher priority than others.

[bole5]

I've tried transfering the file and playing it locally, but unfortunately the problem persists.

Can you please just double check that you have really overwritten the binary with my version?
On my setup with my binary I don't have a problem, while with latest nightly I do experience problems with a/v sync.

You are right about the samba, but I asked you to test file locally to take the networking and protocols out of equation. I can confirm that your video works fine with my binary on my setup over samba on two different media shares (Win7 and OpenWRT).

[bole5]

I'd like to focus on your needing "if (len > free)" in all cases. In the simple case, len is indeed the number of bytes going into m_Buffer. In the more complicated case, you could be re-mapping to more or less channels so you have to use the number of bytes that would result from the remap as that's what's going into m_Buffer. My patch 'should' handle this but since you still see it, there's a hole somewhere and I'd like to understand this better.

Let's just discuss the remapping case, since the simple case seem to work fine. I think that the problem is in using std:min(...) to determine the number of bytes to write to buffer. In case when the free < {recalculated length} you don't read all the available data and that seems not to work correctly. I admit that the problem might be somewhere else as you also return the correct number of accepted bytes, so the receiving side (DVDPlayer?) might be handling number of returned bytes incorrectly.

The sound works well after the following change to AddPackets():

Code:

// call channel remapping routine if available available and required
  if (m_remap.CanRemap() && !m_Passthrough)
  {
    int length, frames;

    int realLen = (len / m_DataChannels) * m_Channels;
    if (realLen > free)
          return 0; // Wait until we can accept all of it
    
    // we might be up or down converting, so convert to number of bytes
    // that we will get out of remapping the channels and see if that fits.
    length = std::min(free, realLen);
    ...

As a consequence the std::min(free, realLen) is not needed and could be replaced by the realLen only...

The example video that can be used for testing this "fix" you can use the video uploaded by lukenukem: http://www65.zippyshare.com/v/44556475/file.html
I will upload the sample video with mono sound as well, but with this fix both videos work fine.


Now to my next observation, which might be related to Locks:

If you keep on presing FFWD real quick, and also pressing RWD you will loose the sound and a/v sync completely. The log will contain the following error:

Code:

ERROR: CIOSCoreAudioDevice::Start: Unable to start device. Error = 0x10000003 (   ).
DEBUG: CDVDPlayerAudio - CDVDMsg::GENERAL_SYNCHRONIZE

If you keep on doing the FFWD/REW without sound and then hit Menu (to go back to the list), the ATV2 will freeze (shell becomes unresponsive) and reboot.

This behaviour I get with the latest version from the trunk, but not from the latest stable. It could be my imagination but I do get far fewer problems when I remove all the locks from the code.

Do we have any particular reason for using potentially unsafe buffer from an external library? Can't we reuse CSliceQueue from CoreAudioRenderer?
IMO, with using CSliceQueue we might simplify the code (by removing locks).

[lukenukem]

Can you please just double check that you have really overwritten the binary with my version?

I am sure, yes. Though I've only overwritten the binary, not any other files. If the other files need to be overwritten as well, I am happy to try again.

But I'd like to note that I don't see how the problem could have changed this significantly if I didn't overwrite the binary. Before the video and audio were sped up, and after the audio was gone and the video was slowed down.

You are right about the samba, but I asked you to test file locally to take the networking and protocols out of equation.

I did actually play the file locally, like you suggested. The network was fully out of the equation. I tested the file several times, and I tested some other similar files as well. They all amount to the slowed down video rendering and the audio dropping out, just 10-15 seconds in.

[davilla]

@bole5 -> git 52e7dc99a7cbd507a5f751364c25b0ad0ecc5bb0

Your changes brought in to trunk, I also switched to mutex usage.

Using CSliceQueue would be best as it is lockless and does not require mutex or critical-sections.

External libraries can be thread-safe or not. depends. av_fifo was used as it is very fast.

git e4f365fa60b5a80c8cd62c7886fc9208e78b0238 is also interesting

[keith]

Thx for help tracking this down bole5!

Few things I appreciate more than someone taking 'patches welcome' to heart.

Look forward to more fixes from you in the future!

[bole5]

I did actually play the file locally, like you suggested. The network was fully out of the equation. I tested the file several times, and I tested some other similar files as well. They all amount to the slowed down video rendering and the audio dropping out, just 10-15 seconds in.

I am sorry that you are still experiencing the problems. The latest fixes to the audio were submitted to the trunk, and Keith's latest unofficial build includes them. Could you perhaps try that release and see if your problem persists?

I can play the video you provided without any problems on my setup (locally and over smb share), so the next step could be updating your ATV2.
If you have the 4.2.1 SHSH blobs saved, could you reinstall 4.2.1 and test again?

[lukenukem]

I'll do a full reinstall with the latest unofficial build, and take it from there.
I appreciate all your help in squashing this bug.

EDIT: After completely removing the old installation, and reinstalling with the latest unofficial build the problem was solved.
I can indeed confirm that the mentioned video files play smoothly with bole5's patch.
Thanks for that guys!

[bole5]

Thanks for that guys!

Well the thanks should go to you and davilla.
You took the time to report the problem and provide logs and samples.