[Claudio.Sjo]

OK, it's not a matter of memory.
Adding swap doesn't help, still I cannot rip the same CD as before.
I make a try with another CD and then will have a look at
reader.Read(stream, 1024);
as it exits during ripping and returns 1.

How do I revert back to the original configuration?
Is it just enough to reload the tar file?

[Claudio.Sjo]

Let's look for the problem, then.
I need the file
https://github.com/xbmc/xbmc/blob/master...DAFile.cpp
to be changed, in order to add some logging.
May you please replace the method CFileCDDA::Read as follows?

Code:

ssize_t CFileCDDA::Read(void* lpBuf, size_t uiBufSize)
{
if (!m_pCdIo || !g_mediaManager.IsDiscInDrive())
{
CLog::Log(LOGERROR, "file cdda: Disk is not in Drive");
return -1;
}
if (uiBufSize > SSIZE_MAX)
uiBufSize = SSIZE_MAX;
// limit number of sectors that fits in buffer by m_iSectorCount
int iSectorCount = std::min((int)uiBufSize / CDIO_CD_FRAMESIZE_RAW, m_iSectorCount);
if (iSectorCount <= 0)
{
CLog::Log(LOGERROR, "file cdda: Sector Count <= 0");
return -1;
}
// Are there enough sectors left to read
if (m_lsnCurrent + iSectorCount > m_lsnEnd)
iSectorCount = m_lsnEnd - m_lsnCurrent;
// The loop tries to solve read error problem by lowering number of sectors to read (iSectorCount).
// When problem is solved the proper number of sectors is stored in m_iSectorCount
int big_iSectorCount = iSectorCount;
while (iSectorCount > 0)
{
int iret = m_cdio->cdio_read_audio_sectors(m_pCdIo, lpBuf, m_lsnCurrent, iSectorCount);
if (iret == DRIVER_OP_SUCCESS)
{
// If lower iSectorCount solved the problem limit it's value
if (iSectorCount < big_iSectorCount)
{
m_iSectorCount = iSectorCount;
}
break;
}
// iSectorCount is low so it cannot solve read problem
if (iSectorCount <= 10)
{
CLog::Log(LOGERROR, "file cdda: Reading %d sectors of audio data starting at lsn %d failed with error code %i", iSectorCount, m_lsnCurrent, iret);
return -1;
}
iSectorCount = 10;
}
m_lsnCurrent += iSectorCount;
return iSectorCount*CDIO_CD_FRAMESIZE_RAW;
}

P.S.
So far I have never experienced another kodi crash, thus I'm willing to say that libcdio has a bug in the logger, and we can keep on going with the patch unless libcdio is fixed properly.

[Milhouse]

How do I revert back to the original configuration?
Is it just enough to reload the tar file?

Yes.

Have you ever thought about building OpenELEC yourself? You seem up to the job...

[Milhouse]

May you please replace the method CFileCDDA::Read as follows?

Give me a patch I can use, and I'll include it in one more test build. My upload bandwidth is severely limited, so uploading test builds frequently is not fun.

and we can keep on going with the patch unless libcdio is fixed properly.

Any potential fix has to go upstream, either to libcdio or OpenELEC, and there needs to be some sign of it being accepted otherwise I'm not going to carry it. There's little benefit having any potential fix in my builds only, and I don't want you becoming reliant on my builds because the fix hasn't gone upstream.

[Claudio.Sjo]

I've always had problems in cross-compiling,
it takes quite a lot to have everything up and running, and I had to give up for instance when dealing with MIPS.
How hard is to have the proper environment for cross-compiling kodi for Raspberry Pi?
Be honest...

About libcdio I'll try to be heard, but so far nobody has even tried to listen to me about the bug.

[Milhouse]

The instructions are pretty straight forward (I managed it...), although it might take an hour or two to get it all setup - the biggest challenge is resolving the dependencies, as they're not all documented so you end up trying a build, failing due to a missing dependency, install it with apt-get, restart the build, fail with another dependency etc. until you finally complete the build...

http://wiki.openelec.tv/index.php?title=...spberry_Pi

[Claudio.Sjo]

Let's try...
but what should I do with the code that I'm going to patch?
As I told you, the patch in libcdio fixes the crash problem, and I'd wish other people to enjoy it.

[Milhouse]

At the very least send it to libcdio. As a consumer of libcdio, OpenELEC might welcome a pull request that adds the patch to their build system, which will be required until libcdio accept/merge the patch and OpenELEC upgrade to the new libcdio version.

[Claudio.Sjo]

I've just updated the ticket towards libcdio
http://trac.kodi.tv/ticket/15757
please help me to make them answer!

[Milhouse]

So you opened a ticket with the one org that isn't able to fix this issue using your current distribution? Primarily this is a libcdio issue, so send the issue there. However this has nothing to do with Kodi as you're using OpenELEC, and OpenELEC uses an external libcdio library. OpenELEC may be willing to support a libcdio patch in the short term.

Also, you say in your ticket that you fixed the issue with the logging change, but I didn't think that was the case (posts #59 and #61) and you wanted to change the CFileCDDA::Read method (still waiting for a patch if you want another test build, unless you're now self-building).

[Claudio.Sjo]

There are 2 problems currently at ripping CD by means of Kodi, both only apply to Raspberry Openelec, because ripping on my Lubuntu based HTPC x86 with Kodi works like a charm.
Problem #1 : Kodi crashes, this seems to be related to the logging in libcdio, and apparently my patch fixes the problem, at least until the real root is found.
Problem #2 : When ripping some CD, the CFileCDDA::Read method returns with a negative value, this happens regularly with some CD (same track, same point) and never with other CDs.

I'm still in the process of building the development environment myself, and will try to find the problem, then I'll try to propose a patch.
(May it be related to the 32bit ARM architecture versus 64bit intel?)

[Milhouse]

(May it be related to the 32bit ARM architecture versus 64bit intel?)

Always possible, wouldn't be the first time.

[Claudio.Sjo]

There's a problem in the logic of this fragment of code belonging to
ssize_t CFileCDDA::Read(void* lpBuf, size_t uiBufSize)

PHP Code:

// The loop tries to solve read error problem by lowering number of sectors to read (iSectorCount).
// When problem is solved the proper number of sectors is stored in m_iSectorCount
int big_iSectorCount = iSectorCount;
while (iSectorCount > 0)
{
int iret = m_cdio->cdio_read_audio_sectors(m_pCdIo, lpBuf, m_lsnCurrent, iSectorCount);
if (iret == DRIVER_OP_SUCCESS)
{
// If lower iSectorCount solved the problem limit it's value
if (iSectorCount < big_iSectorCount)
{
m_iSectorCount = iSectorCount;
}
break;
}
// iSectorCount is low so it cannot solve read problem
if (iSectorCount <= 10)
{
CLog::Log(LOGERROR, "file cdda: Reading %d sectors of audio data starting at lsn %d failed with error code %i", iSectorCount, m_lsnCurrent, iret);
return -1;
}
iSectorCount = 10;
} 


Actually, rather than trying to reduce the number of read sectors as announced in the comment, it barely sets it to 10
and retries, or in case of the number of sectors to read is less than 10 (i.e. close to the end of a track) and a problem occurs, it simply returns.

I will try to make it better and closer to the comment, in the meanwhile I'd wish to tell the team that it's potentially wrong.

[Claudio.Sjo]

Hi Milhouse,
I need a little help.
I did my first building, and all went fine, even though it was longer than expected, but I'm using a poor i5 2400 running 2.5GHz
When I tried to patch libcdio, and then to build again, it seemed to me that it was simply linked, not recompiled.
I tried then "make clean" and, with big surprise, all was extracted from the archives once again.
How can I change the code and build in a reasoneable time?

Thanks,
Claudio

[Milhouse]

"make clean" will clear everything from your build directory. The usual approach is to remove only the relevant libcdio files from the build directory - this means the unpacked source code, and any libcdio entries in the .stamps and image/.stamps directories.

Copy your patch to "package/audio/libcdio/patches" (you'll need to create the patches folder), this way it will be used by all projects. You could alternatively add the patch to "projects/RPi/patches/libcdio", in which case only the RPi project would use the patch when building libcdio.

Then run "make release", and only your libcdio package should need to be rebuilt.