Change the way how extpipe path is verified

[tosiara]

Can we change the way how extpipe verifies if the path is correct?
Currently it opens dummy file to check if the file opens fine:
https://github.com/Motion-Project/motion/blob/master/event.c#L557

  /* Open a dummy file to check if path is correct */
        fd_dummy = myfopen(cnt->extpipefilename, "w");

        /* TODO: trigger some warning instead of only log an error message */
        if (fd_dummy == NULL) {
            /* Permission denied */
            if (errno ==  EACCES) {
                MOTION_LOG(ERR, TYPE_EVENTS, SHOW_ERRNO, "%s: error opening file %s ..."
                           "check access rights to target directory",
                           cnt->extpipefilename);
                return ;
            } else {
                MOTION_LOG(ERR, TYPE_EVENTS, SHOW_ERRNO, "%s: error opening file %s",
                           cnt->extpipefilename);
                return ;
            }

        }

        myfclose(fd_dummy);
unlink(cnt->extpipefilename);

There is some inconvenience with this way, for example, I use a script that monitors the specified directory and copies files to cloud backup. Sometimes randomly the script tries to upload that dummy file which is no longer exist and it generates an error.

Is there other "dry run" way to check if path is valid?
Do we need to validate it at all?

[tosiara]

Any ideas? Am I correct that this code can be removed?

[jogu]

Well I suspect it was added as people's extpipe's were failing in way that didn't make it clear what they had done wrong.

We could (probably) switch to using access() instead I guess, to retain the behaviour:

https://stackoverflow.com/questions/19598497/check-if-a-folder-is-writable

(Though it may miss some cases like weird mis-configurations of NFS where the directory appears to exist and be writeable but we can't actually create a file in it.)

[tosiara]

PR created: #417

If path exists and is writable - silently continues as it was previously
If path does not exist - will try to create it:

[1:ml1] [NTC] [EVT] event_new_video Source FPS 10
[1:ml1] [ERR] [EVT] event_create_extpipe: path not found, trying to create it /home/motion/cam0 ...: No such file or directory
[1:ml1] [NTC] [ALL] create_path: creating directory /home/motion/cam0
[1:ml1] [NTC] [EVT] event_create_extpipe: pipe: x264 - --input-res 640x480 --fps 10 --bitrate 2000 --preset ultrafast --quiet -o /home/motion/cam0/01-20170712_12-01.mp4
[1:ml1] [NTC] [EVT] event_create_extpipe: cnt->moviefps: 10
[1:ml1] [NTC] [EVT] event_newfile: File of type 8 saved to: /home/motion/cam0/01-20170712_12-01
[1:ml1] [NTC] [ALL] motion_detected: Motion detected - starting event 1
^C[1:ml1] [NTC] [ALL] preview_save: different filename or picture only!
[1:ml1] [NTC] [EVT] event_newfile: File of type 1 saved to: /home/motion/cam0/01-20170712_12-01.jpg

If access denied:

[1:ml1] [NTC] [EVT] event_new_video Source FPS 10
[1:ml1] [ERR] [EVT] event_create_extpipe: no write access to target directory /home/motion/cam0: Permission denied
[1:ml1] [NTC] [ALL] motion_detected: Motion detected - starting event 1
^C[1:ml1] [NTC] [ALL] preview_save: different filename or picture only!
[1:ml1] [ERR] [ALL] myfopen: Error opening file /home/motion/cam0/01-20170712_12-02.jpg with mode w: Permission denied
[1:ml1] [ERR] [ALL] put_picture: Can't write picture to file /home/motion/cam0/01-20170712_12-02.jpg - check access rights to target directory
Thread is going to finish due to this fatal error: Permission denied

[Mr-Dave]

Closing via #417

[jasaw]

Please re-open issue.

Pull-request #417 breaks extpipe when movie_filename is set to %Y-%m-%d/%H-%M-%S.
The code checks access to path cnt->conf.filepath, but that does not mean it has access to cnt->extpipefilename, and as a consequence, the directory is not created and ffmpeg fails to write to output file.

I've added additional logs to my motion code to show what I'm talking about.

Motion config:

movie_filename %Y-%m-%d/%H-%M-%S
extpipe ffmpeg -y -f rawvideo -pix_fmt yuv420p -video_size %wx%h -framerate %fps -i pipe:0 -vcodec libx264 -preset ultrafast -f mp4 %f.mp4

Logs:

[1:ml1] [NTC] [EVT] event_new_video: Source FPS 17
[1:ml1] [NTC] [EVT] event_create_extpipe: cnt->conf.filepath: /var/lib/motioneye/Camera1
[1:ml1] [NTC] [EVT] event_create_extpipe: cnt->extpipefilename: /var/lib/motioneye/Camera1/2017-08-08/03-55-33
[1:ml1] [NTC] [EVT] event_create_extpipe: pipe: ffmpeg -y -f rawvideo ...
...
/var/lib/motioneye/Camera1/2017-08-08/03-55-33.mp4: No such file or directory

Motion has access to /var/lib/motioneye/Camera1 directory, so does not create the 2017-08-08 directory, causing extpipe to fail.

[tosiara]

So you are using file name to create subdirectories?

[jasaw]

That is the default motioneye configuration. It would break extpipe for a lot of existing motioneye users if we change the behavior.

I thought it's a legit way of grouping movies by date subfolder.

Correct me if I'm wrong but I don't think it's hard to maintain the original behavior of creating subfolder. The line where we call access to check path, we include the subfolder in the path. What are your thoughts?

[tosiara]

Of course, we can make an additional hack to always create dir structure for whatever file name is. But this is wrong, do you agree? For example, ffmpeg or any other encoder won't create missing structure if you specify output file as "my/new/path/file.mp4"

I would probably prefer to see subdirs in target_dir /my/path/%Y/%M/%d, this way it would work right away. Can you try it for me please?

Since there may be more users affected I will try to make a temporary fix that will work in the old way, but it is better to bring the discussion about this behavior separately

[tosiara]

No, my solution won't work. A fix is needed to always create subdirs...

[jasaw]

I'm not saying which method is right or wrong, just for discussion sake.

This configuration allows for more flexibility, for example:

• target_dir /my/camera/output
• picture_filename pic-%Y-%m-%d/%H-%M-%S
• movie_filename mov-%Y-%m-%d/%H-%M-%S

Movies and pictures can go into separate directories.

[tosiara]

Yes, this makes sense. However, if you made a typo you will end up with unexpected dirs created in random places ;)
Can you try to pull #462 and try it on your setup?
The change will add check of every file name and will create all the needed subdirs

[tosiara]

PR created: #462