[SOLVED] Tried to write something "serious" and have failed once again

michaelk · 07-26-2019, 07:36 AM

Quote:

And also, can I specify argv[3] in execv(), instead of trying to add it to the array for ffmpeg's command-line args? Or is it better to put it into the array instead?

int execv(const char *path, char *const argv[]);

It has to be in the array. the execv function takes two arguments, the first is the path/program to be executed and the second, a pointer to an array of pointers which is the programs command line arguments. The first element of argv is the program name. Here is simple example.

Code:

#include<stdio.h>
#include<stdlib.h>
#include<unistd.h>

main() {
char *temp[] = {NULL,"hello","world",NULL};

temp[0]="hello";
execv("hello",temp);
printf("error");
}

You can use the return value to check if your readConfigFile function found a match in your main function.

GazL · 07-26-2019, 08:02 AM

Normally I'd do something like this:

Code:

char *cmd[] = { "/usr/bin/sleep", "600", NULL };
execv(cmd[0], cmd);

rtmistler · 07-26-2019, 08:30 AM

This would be your synopsis:

Code:

./ffcliFront test testinput out

USAGE: ffcliFront <format_id> <input_file> <output_file>

Seems that the next code block contained the contents of a file named 'testinput':

Code:

test, ffmpeg, ehe, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey
test1, ffmpeg, ehe1, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey
test2, ffmpeg, ehe2, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey

From reading your code, you do not appear to be using the last argument <output_file>.

Can you please post the contents of ffcliFront.conf?
I actually have a suspicion here, which is that instead of reading testinput, that instead my re-copy of what I thought was in testinput, is actually ffcliFront.conf. Y/N?

Quote:

Originally Posted by jsbjsb001

how can I stop it from continuing to loop with the same string specified for the format_id?

In other words, you've asked it to search for the test1 and test2 line terms and it doesn't work, so how to fix this? If a correct guess, then I'll need confirmation of the above questions and points and a bit of time, it may be much later today, just so you know.

To be clear, my questions are:
Are you only using the second argument, argv[1], and none of your other arguments really matter?

Is that block of string you posted, actually the contents of the ffcliFront.conf file?

Quote:

Originally Posted by jsbjsb001

And also, how can I get it to check if the format_id string exists in the config file, and if it doesn't, give the error message and then quit?

Pretty much the same question as above, in my humble opinion. No problem, same comment as above.

Quote:

Originally Posted by jsbjsb001

And also, can I specify argv[3] in execv(), instead of trying to add it to the array for ffmpeg's command-line args? Or is it better to put it into the array instead?

I'll let you know. Michaelk has already said something, but I haven't really reviewed that, because to me a first priority here is getting you to a point where you can write code which will properly parse and search strings. I feel that once you organize strings appropriately, you can then concentrate on constructing an argument list to perform one of the execv() function calls.

After Topic

Code:

    if ( argc < 4 ) {
       fprintf (stderr, "Missing option or filename\n");
       puts("USAGE: ffcliFront <format_id> <input_file> <output_file>");
       return 1;
    }

OK, I'll explain this now since - I got pushback from others about saying something before.

fprintf() to stderr - fine, no problem. This is non-buffered, and hence if you are promptly exiting a program, it's a good idea to get your error post out to the console promptly.

puts() sends information to stdout. This is buffered.
There can possibly be a delay.
No big deal, if the system isn't doing much, no stress, it'll work and work fine, and likely you may never notice that it doesn't work.

My point was that you are constantly mixing functions and using different conventions, probably those of which you really do not understand. Recommend you stick with the same convention such as to perform a fprintf() call to stderr with the usage string. Actually my best recommendation is that you just stick with printf(), use stdout, and once you learn the reasons for why these functions do what they do, then use them appropriately. And also if you use the same function, you will not have any possible sequencing issues with your output.

Within this thread, we do not need to get into a scholarly discussion about I/O buffering.

michaelk · 07-26-2019, 09:10 AM

I might of led you astray a bit if you did not catch my editing. Go back to post #32 and look at my code again (I had fixed some errors) to see how I created the array for the command line arguments to ffmpeg to use for execv.

One feature in my code was to compare format_id to the first value instead of the entire string of the configuration file.

rtmistler · 07-26-2019, 09:26 AM

Quote:

Originally Posted by jsbjsb001

how can I stop it from continuing to loop with the same string specified for the format_id?

You need a break statement after line 71. At the bottom of the successful if() statement, but past that inner while() block.

Quote:

Originally Posted by jsbjsb001

And also, how can I get it to check if the format_id string exists in the config file, and if it doesn't, give the error message and then quit?

Per my interpretation here, you need to add a flag which indicates success and test that flag after you've exited the while loop. If the flag indicates false, then it was never found.

jsbjsb001 · 07-26-2019, 11:45 PM

Thanks again guys!

My question about execv() was that; can I specify an array for the arguments to be passed to ffmpeg and argv[3], so two arrays not just one? It's ok if I can't, but in that case I'll have to get argv[3] into the ffargs array, on the same line as the rest of it's contents.

Quote:

Originally Posted by rtmistler

This would be your synopsis:

Thank you RT.

Quote:

Seems that the next code block contained the contents of a file named 'testinput':

Code:

test, ffmpeg, ehe, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey
test1, ffmpeg, ehe1, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey
test2, ffmpeg, ehe2, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey

No, "testfile" is just an empty file I created because the program expects 3 arguments after the name of the program, and it checks whether or not that file that argument refers to exists. The reason there was because, if the input file doesn't exist, then ffmpeg itself is going to complain that you didn't specify an input file - therefore it's pointless for the program to go any further (being the reason it will quit if you don't specify one).

Quote:

From reading your code, you do not appear to be using the last argument <output_file>.

At the moment no, but once we can get this bit (parsing the config file) sorted out, the final argument (argv[3]) will be used as the final string in the arguments passed to ffmpeg itself (to give it the output filename).

Quote:

Can you please post the contents of ffcliFront.conf?

What you quoted above is the content of "ffcliFront.conf".

Quote:

I actually have a suspicion here, which is that instead of reading testinput, that instead my re-copy of what I thought was in testinput, is actually ffcliFront.conf. Y/N?In other words, you've asked it to search for the test1 and test2 line terms and it doesn't work, so how to fix this? If a correct guess, then I'll need confirmation of the above questions and points and a bit of time, it may be much later today, just so you know.

No, "testinput" is like I said above, just a dummy (empty) file so that my program doesn't complain and quit.

Quote:

To be clear, my questions are:
Are you only using the second argument, argv[1], and none of your other arguments really matter?

Is that block of string you posted, actually the contents of the ffcliFront.conf file?

No, as above the third argument also matters - but again, my program doesn't actually do anything with the content of that third argument, other than puts it into the array after the "-i" for the name of the input file to be passed to ffmpeg itself.

Quote:

...because to me a first priority here is getting you to a point where you can write code which will properly parse and search strings. I feel that once you organize strings appropriately, you can then concentrate on constructing an argument list to perform one of the execv() function calls.

Yes, that was my plan - to get it parsing the config correctly, then move on to execv(). So yes, I agree with you.

Quote:

After Topic

Code:

    if ( argc < 4 ) {
       fprintf (stderr, "Missing option or filename\n");
       puts("USAGE: ffcliFront <format_id> <input_file> <output_file>");
       return 1;
    }

OK, I'll explain this now since - I got pushback from others about saying something before.

fprintf() to stderr - fine, no problem. This is non-buffered, and hence if you are promptly exiting a program, it's a good idea to get your error post out to the console promptly.

puts() sends information to stdout. This is buffered.
...
Within this thread, we do not need to get into a scholarly discussion about I/O buffering.

I'm really not quite sure what you mean by puts() being buffered. But I've changed the puts() statement to an fprintf() statement, printing to stdout.

I tried what you said in post #80, and the break statement fixed the problem of it continuing to loop, if you have a similar name for the format_id - so that problem appears to be fixed. As, if I specify "test" as the format_id it no longer continues looping the rest of the lines in the config file (ffcliFront.conf). And still gives the correct result if I use "test1" or "test2" as the second argument (argv[1]) instead. But while I've almost fixed the second problem of getting it to display an error message and quit if you specify a string for the format_id that matches none of the lines in the config file; I'm having a similar problem with that as I was with it continuing to loop if you specified a similar name/string.

So for example; if I specify "tes" as the second argument, it still uploads the line from the config file that starts with "test", but if I specify "tes6"/something completely different, it works, and displays the error message, then quits as intended. So same sort of problem as before, but I can't figure out how to use the bool value to fix it. I've tried a number of different things, but the code below is as close as I could get.

Quote:

Originally Posted by michaelk

I might of led you astray a bit if you did not catch my editing. Go back to post #32 and look at my code again (I had fixed some errors) to see how I created the array for the command line arguments to ffmpeg to use for execv.

One feature in my code was to compare format_id to the first value instead of the entire string of the configuration file.

Thank you michaelk. I had a look at your code, and it does make a lot more sense to me now. Thanks again for your help!

Anyways, here's my "almost" correctly parsing code;

Code:

// includes
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <stdbool.h>

char readConfigFile(char *argv[]);
//char readConfigFile(char *filename, char *formatID);
char execFfmpeg(char *argv[], char ffargs[]);

// main function
int main(int argc, char *argv[]) {
  
    if ( argc < 4 ) {
       fprintf (stderr, "Missing option or filename\n");
       fprintf(stdout, "USAGE: ffcliFront <format_id> <input_file> <output_file>\n");
       return 1;
    }
          
    readConfigFile(argv);
    //readConfigFile(argv[1], argv[2]);

    return 0;
    
}

//char readConfigFile(char *filename, char *formatID) {
char readConfigFile(char *argv[]) {

    char ffFormatID[128];
    char ffargs[128];
    char *tokenPtr;
    bool foundStr = false;
    
    // check if input file exists
    FILE *inputfile;
        
    if ( (inputfile = fopen(argv[2], "r")) == NULL ) {
       fprintf(stderr, "Input file not found, exiting.\n");
       return 1;        
    }
    fclose(inputfile);
       
    // read config file
    FILE *configfile;
      
       if ( (configfile = fopen("ffcliFront.conf", "r")) == NULL ) {
       fprintf(stderr, "Configuration file not found, exiting.\n");
       return 1;        
    }
    
    printf("argv[1] = %s\n", argv[1]);
       
    if ( (configfile = fopen("ffcliFront.conf", "r")) != NULL) {
       while( fgets(ffFormatID, 128, configfile ) != NULL) {
             printf("Line from config file: %s\n", ffFormatID);
	     if ( (strstr(ffFormatID, argv[1]) ) != NULL) {
	        foundStr = true;
	        printf("Found string: %s\n", ffFormatID);
		tokenPtr = strtok(ffFormatID, ",");
		printf("tokenPtr = %s\n", tokenPtr);
                strcpy(ffargs, "-i ");
		strcat(ffargs, argv[2]);
		 
                while( tokenPtr != NULL ) {
		      tokenPtr = strtok(NULL, ",");  
	              printf("Token: %s\n", tokenPtr);
	              if (tokenPtr) {
	                 printf("tokenPtr = %s\n", tokenPtr);
	                 strcat(ffargs, tokenPtr);
		      }
			 
		}
		break;
	        //  strcat(ffargs, argv[3]); 
	     }
	       	      
       }
       if (foundStr == false) {
          fprintf(stderr, "\"%s\" format_id not found in configuration file\n", argv[1]);
          return 1;
       }
       
    }
    fclose(configfile);
             
    printf("ffargs = %s\n", ffargs);
        
    // pass char array with ffmpeg args & call execFfmpeg() fuction with args from config file
   // execFfmpeg(argv, ffargs);
 
    return 0;
}      

//char execFfmpeg(char *argv[], char ffargs[]) {
  
    //char *ffcmdline[128];  
     
    //printf("%s\n", ffargs); 
    // run fmpeg with args from config file & filename supplied to this program. 
    // execv ("/usr/bin/ffmpeg", argv[3], ffcmdline);
	
    //return 0;
//}

ntubski · 07-27-2019, 06:43 AM

Quote:

Originally Posted by jsbjsb001

I'm really not quite sure what you mean by puts() being buffered. But I've changed the puts() statement to an fprintf() statement, printing to stdout.

Why not send both messages to the same stream? Seems potentially confusing to split them up.

GazL · 07-27-2019, 06:52 AM

Quote:

Originally Posted by jsbjsb001

My question about execv() was that; can I specify an array for the arguments to be passed to ffmpeg and argv[3], so two arrays not just one?

No, you can only pass 1 array of args to execv(). You will have to add your extra arguments to that array.

Some comments to help you on your way:

Checking the existence of the input file has nothing to do with reading the config file: move that code into to main() just after you check the number of args.

You're still opening the config file twice in the readConfigFile function.

Code:

   if ( (configfile = fopen("ffcliFront.conf", "r")) != NULL) {

is completely unnecessary, and in fact wrong as you've already opened and checked for NULL above: remove the if block that is around your while loop.

ffFormatID is a bad variable name for something that holds a line from your config file: I would have gone with "configLine".

IMO, your program has structural problems, but lets leave that discussion for later as I don't want to interfere too much at this point.

jsbjsb001 · 07-27-2019, 08:34 AM

Quote:

Originally Posted by ntubski

Why not send both messages to the same stream? Seems potentially confusing to split them up.

My thinking was that the "Usage" statement wasn't an "error message" per se, but I guess it does make sense to send both messages to the same place. So I got rid of the second fprintf() statement, and just put a newline between both messages in the same fprintf() statement. Thanks ntubski.

Quote:

Originally Posted by GazL

No, you can only pass 1 array of args to execv(). You will have to add your extra arguments to that array.

Thanks GazL. I've used strcat() to add the output filename to the ffargs array, but it stills appears to put it on a newline for some reason. So I guess (hopefully) that can be alright given I'd have to setup a pointer array for the args, for execv().

But hopefully I can resolve the last remaining problem (I think) of it only giving an error and quitting, if the format_id is completely different to the first string on each line in the config file. Because as I was saying above, if I specify "tes"/the same characters, even if it's not exactly the same as the first string of one of the lines in the config file, it still uploads the line starting with "test", instead of quitting with an error message like it's supposed to.

Quote:

Some comments to help you on your way:

Checking the existence of the input file has nothing to do with reading the config file: move that code into to main() just after you check the number of args.

I think I had that block in main() before, but someone said to put it into the readConfigFile() function. But I do agree with you, as it didn't make sense to me to have it in that function either. So I've moved it back to main(), given it's only there to make sure the input file actually exists - as it's pointless for the program to continue if it doesn't exist.

Quote:

You're still opening the config file twice in the readConfigFile function.

Code:

   if ( (configfile = fopen("ffcliFront.conf", "r")) != NULL) {

is completely unnecessary, and in fact wrong as you've already opened and checked for NULL above: remove the if block that is around your while loop.

Didn't even notice that, good spot. So I got rid of that if statement like you said.

Quote:

ffFormatID is a bad variable name for something that holds a line from your config file: I would have gone with "configLine".

IMO, your program has structural problems, but lets leave that discussion for later as I don't want to interfere too much at this point.

Yeah, I was thinking the same, and was thinking about changing it's name - so I changed it to what you suggested, given it seems like the most accurate description of what that array is for.

You're welcome to offer any advice/suggestions GazL - particularly when you always offer good suggestions/advice.

michaelk · 07-27-2019, 09:15 AM

Quote:

test, ffmpeg, ehe, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey
test1, ffmpeg, ehe1, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey
test2, ffmpeg, ehe2, te, teyeyyes, y, eye, yeh, eyh, eeee, eyeye, e, ey

strstr finds the first occurrence of the substring i.e. argv[1] in ffFormatID not an exact match. If argv[1]=tes then strstr(ffFormatID, argv[1]) will always be true for every line. If you exit your loop on the first found occurence then it will always be line one. If you use something more realistic in your configuration file when running your programing then extra verification steps are not necessary. Otherwise you need an additional code to verify an exact match.

GazL · 07-27-2019, 12:01 PM

You also have to watch out for partial matches. In my version, rather than use strstr() I used strncasecmp() to find the match and then I checked that the next character in the config line was a space in order to avoid a partial match.

I assume this is what you're intending to do?

Code:

$ cat ffclifront.conf
ogg -codec:a libvorbis -qscale:a 3
mp3-cbr -codec:a libmp3lame -b:a 320k
mp3 -codec:a libmp3lame -qscale:a 2
test -1 -2 -3 -4 -5 -6 -7 -8 -9 -10 -11 -12
$ ./ffclifront mp3 infile outfile
config line found: mp3 -codec:a libmp3lame -qscale:a 2

arg: /usr/bin/ffmpeg
arg: -i
arg: infile
arg: -codec:a
arg: libmp3lame
arg: -qscale:a
arg: 2
arg: outfile
$ ./ffclifront MP3-CBR infile outfile
config line found: mp3-cbr -codec:a libmp3lame -b:a 320k

arg: /usr/bin/ffmpeg
arg: -i
arg: infile
arg: -codec:a
arg: libmp3lame
arg: -b:a
arg: 320k
arg: outfile
$

If you want to see my code ask me and I'll post it, but as this is a programming exercise you'll learn a lot through trial and error.

The only problem (that I'm aware of) with my code now is dealing with a quoted string in the conf file as I'm not sure it can be done with strtok() as you don't have visibility of the internal pointer it uses. Might be easier with something like strsep() but that's not a C standard function (if you care about that).

rtmistler · 07-27-2019, 03:04 PM

Quote:

Originally Posted by jsbjsb001

But hopefully I can resolve the last remaining problem (I think) of it only giving an error and quitting,

The input file is just something which you read serially.

If you'll have an input file syntax, then you can check, line by line that each line meets valid forms. Example: Starts with a specific range of keywords, or IS a word and does not include any punctuation or numbers/symbols. Stuff like that. If you see a bad first word in any line, you move to the next as soon as possible.

I see no point in checking the file more than once. I don't get why you wish to see one bad term and hope to exit fast. It's a computer, trust me, it's fast.

Suggestion for your file, any line starting with #, treat it as a comment and skip parsing the line. Same point I'm making with this post, check the first word, if it's not a term you'd expect, skip parsing the line.

rtmistler · 07-27-2019, 04:42 PM

The recommendation to use strcasecmp() is one way to check the search term against the first term in a line from the file.

Also, you can see if the strlen() for your search term matches the length of the first word in a line. You can save the strlen () of the search term to a variable, so you're only calling it once per line check.

FYI about buffering vs not. Stdout "can" be buffered, which means that something non-buffered, like stderr gets forced to the console first. Say you have a function called, there's some sort of status about it not working. So you send just "Error" to stderr, and use puts() or printf() to output more information. But you also return to the main() and the function returns an error. Say also that main() checks that error, and also sends a string to stderr, and exits. It is "possible" that things could be out of sequence because a non-buffered I/O like stderr gets priority.

Once again, "no huge deal" until such point as you encounter it, then turn it into this huge question, on top of another question. Truth is, I raised this all, but the reason for it was to avoid a possible rarely occurring thing to pop up and really sidetrack you.

GazL · 07-27-2019, 05:08 PM

Quote:

Originally Posted by rtmistler

Suggestion for your file, any line starting with #, treat it as a comment and skip parsing the line.

Good idea. I just added that to my solution.

GazL · 07-27-2019, 07:16 PM

Ok, I've now got quoted arguments to work using strtok() so I can now include an argument containing spaces.

Code:

$ ./ffclifront test3 Makefile output
config line found: test3 -1 -2 "wibble wobble froop" -4
arg: /usr/bin/ffmpeg
arg: -i
arg: Makefile
arg: -1
arg: -2
arg: wibble wobble froop
arg: -4
arg: output
$

However, it's still doesn't cope with an argument like option="wibble wobble froop". I'm starting to think strtok() is just too blunt a tool for this job, so it looks like I'll have to roll my own splitting function. I might have a go at that tomorrow.