LinuxTV Patchwork VDR 1.7 FreeBSD segfault when cutting recordings

login
register
mail settings
Submitter Klaus Schmidinger
Date March 18, 2013, 9:50 a.m.
Message ID <5146E353.5030901@tvdr.de>
Download mbox | patch
Permalink /patch/17406/
State New
Headers show

Comments

Klaus Schmidinger - March 18, 2013, 9:50 a.m.
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

Patch

--- cutter.c	2013/02/17 14:11:03	2.24
+++ cutter.c	2013/03/18 09:40:49
@@ -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)) {

Privacy Policy