Verification required (dodgy programmer @ work)

[StainlessS]

Hi guys, just need some assurance that I'm not doing anything terribly sinful in the plug I'm putting together, the possibly
problematic bit concerns returning a string to Avisynth and handing over management of it.
The code of concern is hi-lited, the rest is still a work in progress and need not attract comment.
Also, Am I correct in thinking that the env->SaveString func makes a copy of the string and returns the new memory buffer
to the caller which can then be handed back to Avisynth and the
original buffer freed.
I would appreciate some feedback on this as stoney silence is not very reassuring.

Here's the code as it currently stands:


AVSValue __cdecl Create_RT_ReadTxtFromFile(AVSValue args, void* user_data, IScriptEnvironment* env) {
enum{BFSZ=512};
char bf[BFSZ+2]; // with a little spare
const char *filename=args[0].AsString();
int Lines=args[1].AsInt(0); // Allow user set default=0
FILE *fp;
unsigned char c,*p,*s,*pbf;
long flen,chk,wr,line;
if(!(fp = fopen(filename, "r"))) {env->ThrowError("RT_ReadTxtFromFile: Cannot Open file %s",filename);}
if((fseek(fp, 0, SEEK_END)!=0) || ((flen=ftell(fp)) == -1L)) { fclose(fp);
env->ThrowError("RT_ReadTxtFromFile: Cannot seek in file %s",filename);}
rewind(fp);
if ((chk=3)>flen) chk=flen;
if(chk==0) { fclose(fp); bf[0]=0; return env->SaveString(bf);} // Return "" to Avisynth
wr = fread(bf, 1, chk, fp);
if(wr != chk) {fclose(fp);env->ThrowError("RT_ReadTxtFromFile: Cannot read from file %s",filename);}
if(chk==3 && bf[0]==0xEF && bf[1]==0xBB && bf[2]==0xBF) {
fclose(fp); env->ThrowError("RT_ReadTxtFromFile: UTF-8 text files are not supported, "
"re-save with ANSI encoding! : '%s'", filename);}
if((chk>=2 && ((bf[0]==0xFF && bf[1]==0xFE) || (bf[0]==0xFE && bf[1]==0xFF)))) {
fclose(fp); env->ThrowError("RT_ReadTxtFromFile: UTF-8 text files are not supported, "
"re-save with ANSI encoding! : '%s'", filename);}
rewind(fp);
if((pbf = new unsigned char[flen])==NULL)
{fclose(fp); env->ThrowError("RT_ReadTxtFromFile: Cannot allocate memory for %s",filename);}
p=pbf;
if(Lines <= 0) Lines = 0x7FFFFFFF;
line = 0;
while(line < Lines && (fgets(bf, BFSZ,fp ) != NULL)) { // will halt at first Nul byte if bin file
++line;
s = (unsigned char *)bf; // start of input buffer
while(c = *s) {
if(c>=' ' && c <= 126) { // All ASCII printable incl SPACE
*p++=c;
} else if(c=='\n' || c=='\r') {
*p++='\n'; // Output single '\n'
if((c=='\n' && s[1]=='\r') || (c=='\r' && s[1]=='\n')) // Allow skip of matched pairs CR/LF or LF/CR
++s; // Skip to 1st of match pair EOL
} else { // Just allow control codes and non Ascii through, maybe change in future, perhaps user selectable
*p++ = c;
}
++s; // Next char in input buffer
}
}
*p='\0'; // Nul term sentinel
fclose(fp);
AVSValue ret = env->SaveString((const char*)pbf); // Get pointer to Avisynth saved string
delete [] pbf; // Delete original buffer
return ret; // Return Avisynth's copy to Avisynth
}

EDITED:

and the calling script

TS=RT_ReadTxtFromFile("D:\avs\bank\mcdegrain.avs")
RT_debug(TS)
TS="" # Hopefully this will deallocate the string.
EDIT: This would have been a better choice: TS=0



And the output in RT_Stats RT_Debug()

00000004 02:32:34 RT_Debug:
00000005 02:32:34 RT_Debug: Function MCDegrain(clip c, int "frames")
00000006 02:32:34 RT_Debug: { # By Didee, http://forum.doom9.org/showthread.php?p=1508289#post1508289
00000007 02:32:34 RT_Debug:
00000008 02:32:34 RT_Debug: frames = default(frames, 2)
00000009 02:32:34 RT_Debug: bs = (c.width>960) ? 16 : 8
00000010 02:32:34 RT_Debug: super = c.MSuper(pel=2, sharp=1)
00000011 02:32:34 RT_Debug: backward_vec3 = MAnalyse(super, isb = true, delta = 3, blksize=bs, overlap=bs/2)
00000012 02:32:34 RT_Debug: backward_vec2 = MAnalyse(super, isb = true, delta = 2, blksize=bs, overlap=bs/2)
00000013 02:32:34 RT_Debug: backward_vec1 = MAnalyse(super, isb = true, delta = 1, blksize=bs, overlap=bs/2)
00000014 02:32:34 RT_Debug: forward_vec1 = MAnalyse(super, isb = false, delta = 1, blksize=bs, overlap=bs/2)
00000015 02:32:34 RT_Debug: forward_vec2 = MAnalyse(super, isb = false, delta = 2, blksize=bs, overlap=bs/2)
00000016 02:32:34 RT_Debug: forward_vec3 = MAnalyse(super, isb = false, delta = 3, blksize=bs, overlap=bs/2)
00000017 02:32:34 RT_Debug: (frames<=0) ? c :\
00000018 02:32:34 RT_Debug: (frames==1) ? c.MDegrain1(super, backward_vec1,forward_vec1,thSAD=400) :\
00000019 02:32:34 RT_Debug: (frames==2) ? c.MDegrain2(super, backward_vec1,forward_vec1,backward_vec2,forward_vec2,thSAD=400) :\
00000020 02:32:34 RT_Debug: c.MDegrain3(super, backward_vec1,forward_vec1,backward_vec2,forward_vec2,backward_vec3,forward_vec3,thSAD=400)
00000021 02:32:34 RT_Debug: return(last)
00000022 02:32:34 RT_Debug: }
00000023 02:32:34 RT_Debug:
00000024 02:32:34 RT_Debug: Function MCDegrainSharp(clip c, int "frames", float "bblur", float "csharp", bool "bsrch")
00000025 02:32:34 RT_Debug: { # Based on MCDegrain By Didee, http://forum.doom9.org/showthread.php?t=161594
00000026 02:32:34 RT_Debug: # Also based on DiDee observations in this thread: http://forum.doom9.org/showthread.php?t=161580
00000027 02:32:34 RT_Debug: # "Denoise with MDegrainX, do slight sharpening where motionmatch is good, do slight blurring where motionmatch is bad"
00000028 02:32:34 RT_Debug: # In areas where MAnalyse cannot find good matches, the blur() will be dominant.
00000029 02:32:34 RT_Debug: # In areas where good matches are found, the sharpen()'ed pixels will overweight the blur()'ed pixels
00000030 02:32:34 RT_Debug: # when the pixel averaging is performed.
00000031 02:32:34 RT_Debug: frames = default(frames, 2)
00000032 02:32:34 RT_Debug: bblur = default(bblur, 0.6)
00000033 02:32:34 RT_Debug: csharp = default(csharp, 0.6)
00000034 02:32:34 RT_Debug: bsrch = default(bsrch, true)
00000035 02:32:34 RT_Debug: bs = (c.width>960) ? 16 : 8
00000036 02:32:34 RT_Debug:
00000037 02:32:34 RT_Debug: c2 = c.blur(bblur)
00000038 02:32:34 RT_Debug: super = bsrch ? c2.MSuper(pel=2, sharp=1) : c.MSuper(pel=2, sharp=1)
00000039 02:32:34 RT_Debug: super_rend = c.sharpen(csharp).MSuper(pel=2, sharp=1,levels=1)
00000040 02:32:34 RT_Debug: backward_vec3 = MAnalyse(super, isb = true, delta = 3, blksize=bs, overlap=bs/2)
00000041 02:32:34 RT_Debug: backward_vec2 = MAnalyse(super, isb = true, delta = 2, blksize=bs, overlap=bs/2)
00000042 02:32:34 RT_Debug: backward_vec1 = MAnalyse(super, isb = true, delta = 1, blksize=bs, overlap=bs/2)
00000043 02:32:34 RT_Debug: forward_vec1 = MAnalyse(super, isb = false, delta = 1, blksize=bs, overlap=bs/2)
00000044 02:32:34 RT_Debug: forward_vec2 = MAnalyse(super, isb = false, delta = 2, blksize=bs, overlap=bs/2)
00000045 02:32:34 RT_Debug: forward_vec3 = MAnalyse(super, isb = false, delta = 3, blksize=bs, overlap=bs/2)
00000046 02:32:34 RT_Debug: (frames<=0) ? c :\
00000047 02:32:34 RT_Debug: (frames==1) ? c2.MDegrain1(super_rend, backward_vec1,forward_vec1,thSAD=400) :\
00000048 02:32:34 RT_Debug: (frames==2) ? c2.MDegrain2(super_rend, backward_vec1,forward_vec1,backward_vec2,forward_vec2,thSAD=400) :\
00000049 02:32:34 RT_Debug: c2.MDegrain3(super_rend, backward_vec1,forward_vec1,backward_vec2,forward_vec2,backward_vec3,forward_vec3,thSAD=400)
00000050 02:32:34 RT_Debug: return(last)
00000051 02:32:34 RT_Debug: }
00000052 02:32:34 RT_Debug:
00000053 02:32:34 RT_Debug:


The New plug is to read in a text file and return it in a string,
and work being done on RT_Debug, will parse a string for EOL codes and output to OutputDebugString.

EDIT:
RT_Stats v1.02, below of from mediafire account in sig:
http://forum.doom9.org/showthread.php?t=165479
EDIT: Above link fixed.

[IanB]

env->SaveString(data) copies the string data into the strings save space of the current IScriptEnvironment. The string save space is released only when the current IScriptEnvironment is deleted.

So no,
TS="" # Hopefully this will deallocate the string.
will not release the memory formally pointed to by TS, it just sets TS to point to another string, i.e. "".

For text string data, env->SaveString(data), is a simple and convenient way to do semi-static allocation.

For more complex resources you can register a clean up handler with env->AtExit(). Again this is run when the current IScriptEnvironment is deleted.


Not sure what you are trying to achieve here, but it get a hint you are still struggling with the distinction between compile time and execution time. The goal of compiling a script is to produce a final complex object with a GetFrame() method and a GetAudio() method that the host application can call to retrieve video frames and audio data respectively. Having arbitrary things happen while the script is compiling can be a useful thing, but it should not distract from the final goal. If your final goal is not to have something make GetFrame() and/or GetAudio() calls, then Avisynth is possibly not the right tool for the task.

[StainlessS]

Thank you for you answer, must assume that all internally managed strings are likewise accumulated until release of current script environment.

The purpose of the RT stuff is ultimately to provide a more automatic solution to video format conversion, so that operations usually
done by a person can be automated prior to frame serving. Being able to eg extract the first few lines from a d2v file to ascertain
eg full frame Display Aspect Ratio or whatever, can be useful and reduces the amount of that personal touch needed.

I dont think its such a bad thing to have options.

Again, thanks for the response.

EDIT: I guess that NOT using env->SaveString and just forgetting altogether about delete-ing the space is pretty
much OK too and will be removed when ALL Environments disappear.
Am now happier I think I fully understand, although a little memory garbage collection might be a worth considering.
EDIT:
env->SaveString actually means, "delete string When current environment closes down". ?

[IanB]

must assume that all internally managed strings are likewise accumulated until release of current script environment.Yes.
EDIT: I guess that NOT using env->SaveString and just forgetting altogether about delete-ing the space is pretty
much OK too and will be removed when ALL Environments disappear.Not deleting memory is called leaking and is most likely a bad thing. Avisynth is a library and at the beck and call of the parent application. Think along the lines of VirtualDub or other program running a big batch of queued jobs. It does not exit between jobs. Each IScriptEnvironment is created at the start and deleted at the end of each job.
Am now happier I think I fully understand, although a little memory garbage collection might be a worth considering.Yeah, maybe reference counting strings could be done, but would be a lot of work to change the code. Even for excessive scripts the string space is usually 100's of Kilobytes, compared to Megabytes for video frames.
EDIT: env->SaveString actually means, "delete string When current environment closes down". ?
Yes, or "Save the string until the IScriptEnvironment closes down".

[StainlessS]

Thanks. :)