VDR 1.7 FreeBSD segfault when cutting recordings
Commit Message
On 18.03.2013 10:10, Gerhard Brauer wrote:
> On Mon, Mar 18, 2013 at 09:13:02AM +0100, Klaus Schmidinger wrote:
>> On 18.03.2013 08:41, Gerhard Brauer wrote:
>>>
>>> The cutting process now works without segfaulting or unbehavior exit
>>> of the vdr process itself - but during cutting i got A LOT of "frame
>>> larger than buffer" x greater then 8 (8 is fix in all messages).
>>> -----------
>>> Mar 18 08:30:12 s01 vdr: [50364416] ERROR: frame larger than buffer (17392 > 8)
>>> Mar 18 08:30:12 s01 vdr: [50364416] ERROR: frame larger than buffer (17514 > 8)
>>> Mar 18 08:30:12 s01 vdr: [50364416] video cutting thread ended (pid=33513, tid=50364416)
>>> Mar 18 08:30:13 s01 vdr: [50361344] info: Schnitt beendet
>>> -----------
>>> The cutting ends now always normally.
>>
>> I guess the problem is that Juergen has allocated the buffers on the
>> heap, but did not change the places where the buffer size is determined
>> using sizeof(buffer) and sizeof(buffer2). If you replace these with
>> MAXFRAMESIZE it should work.
>
> Yes, this works fine. I've had to add this to Juergen's patch:
> ---------
> @@ -108,7 +119,7 @@
> CurrentFileNumber = FileNumber;
> }
> if (fromFile) {
> - int len = ReadFrame(fromFile, buffer, Length, sizeof(buffer));
> + int len = ReadFrame(fromFile, buffer, Length, MAXFRAMESIZE);
> if (len < 0) {
> error = "ReadFrame";
> break;
> @@ -193,7 +204,7 @@
> if (FileNumber != CurrentFileNumber)
> fromFile = fromFileName->SetOffset(FileNumber, FileOffset);
> if (fromFile) {
> - int len = ReadFrame(fromFile, buffer2, Length2, sizeof(buffer2));
> + int len = ReadFrame(fromFile, buffer2, Length2, MAXFRAMESIZE);
> if (len >= 0 && len == Length2)
> CheckForSeamlessStream = true;
> }
> ---------------
>
> First I'm (me.noob <g>!) tried it with sizeof(MAXFRAMESIZE), the
> result was not what i expected ;-)
> But i consulted K&R, both mean heap and stack were realy nice
> things... Hej, i'm a ruby guy, we have garbage collectors ;-)
>
> I think Juergen will make a "final patch", i don't know if above
> changes are well for all platforms, maybe a #ifdef __FreeBSD__ is
> needed.
> But for me: it works!
>
> Thanks for a wonderfull piece oft software!
>>
>> Klaus
>>
>
> Regards
> Gerhard
>
>>>> --- cutter.c.orig
>>>> +++ cutter.c
>>>> @@ -83,7 +83,18 @@ void cCuttingThread::Action(void)
>>>> int LastIFrame = 0;
>>>> toMarks.Add(0);
>>>> toMarks.Save();
>>>> +#ifdef __FreeBSD__
>>>> + // XXX save thread stack space
>>>> + uchar *buffer = MALLOC(uchar, MAXFRAMESIZE);
>>>> + uchar *buffer2 = MALLOC(uchar, MAXFRAMESIZE);
>>>> + if (buffer == NULL || buffer2 == NULL) {
>>>> + free(buffer);
>>>> + error = "malloc";
>>>> + return;
>>>> + }
>>>> +#else
>>>> uchar buffer[MAXFRAMESIZE], buffer2[MAXFRAMESIZE];
>>>> +#endif
>>>> int Length2;
>>>> bool CheckForSeamlessStream = false;
>>>> bool LastMark = false;
>>>> @@ -216,6 +227,10 @@ void cCuttingThread::Action(void)
>>>> }
>>>> }
>>>> Recordings.TouchUpdate();
>>>> +#ifdef __FreeBSD__
>>>> + free(buffer);
>>>> + free(buffer2);
>>>> +#endif
>>>> }
>>>> else
>>>> esyslog("no editing marks found!");
>>>> --- thread.c.orig
>>>> +++ thread.c
>>>> @@ -242,7 +242,7 @@ void cThread::SetPriority(int Priority)
>>>> void cThread::SetIOPriority(int Priority)
>>>> {
>>>> #ifdef __FreeBSD__
>>>> - esyslog("ERROR: syscall(SYS_ioprio_set ...) unsupported on FreeBSD");
>>>> + // esyslog("ERROR: syscall(SYS_ioprio_set ...) unsupported on FreeBSD");
>>>> #else
>>>> if (syscall(SYS_ioprio_set, 1, 0, (Priority & 0xff) | (2 << 13)) < 0) // best effort class
>>>> LOG_ERROR;
For completeness, here's the patch as it will go into the current developer version.
Klaus
@@ -362,11 +362,22 @@
return true;
}
+class cHeapBuffer {
+private:
+ uchar *buffer;
+public:
+ cHeapBuffer(int Size) { buffer = MALLOC(uchar, Size); }
+ ~cHeapBuffer() { free(buffer); }
+ operator uchar * () { return buffer; }
+ };
+
bool cCuttingThread::FramesAreEqual(int Index1, int Index2)
{
+ cHeapBuffer Buffer1(MAXFRAMESIZE);
+ cHeapBuffer Buffer2(MAXFRAMESIZE);
+ if (!Buffer1 || !Buffer2)
+ return false;
bool Independent;
- uchar Buffer1[MAXFRAMESIZE];
- uchar Buffer2[MAXFRAMESIZE];
int Length1;
int Length2;
if (LoadFrame(Index1, Buffer1, Independent, Length1) && LoadFrame(Index2, Buffer2, Independent, Length2)) {
@@ -386,12 +397,14 @@
void cCuttingThread::GetPendingPackets(uchar *Data, int &Length, int Index)
{
+ cHeapBuffer Buffer(MAXFRAMESIZE);
+ if (!Buffer)
+ return;
bool Processed[MAXPID] = { false };
cPacketStorage PacketStorage;
int64_t LastPts = lastVidPts + delta;// adding one frame length to fully cover the very last frame
Processed[patPmtParser.Vpid()] = true; // we only want non-video packets
for (int NumIndependentFrames = 0; NumIndependentFrames < 2; Index++) {
- uchar Buffer[MAXFRAMESIZE];
bool Independent;
int len;
if (LoadFrame(Index, Buffer, Independent, len)) {
@@ -534,8 +547,12 @@
bool SeamlessBegin = LastEndIndex >= 0 && FramesAreEqual(LastEndIndex, BeginIndex);
bool SeamlessEnd = NextBeginIndex >= 0 && FramesAreEqual(EndIndex, NextBeginIndex);
// Process all frames from BeginIndex (included) to EndIndex (excluded):
+ cHeapBuffer Buffer(MAXFRAMESIZE);
+ if (!Buffer) {
+ error = "malloc";
+ return false;
+ }
for (int Index = BeginIndex; Running() && Index < EndIndex; Index++) {
- uchar Buffer[MAXFRAMESIZE];
bool Independent;
int Length;
if (LoadFrame(Index, Buffer, Independent, Length)) {