Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

smugmug_fs: handle SmugMug shenanigans that sometimes movie files #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

durin42
Copy link
Contributor

@durin42 durin42 commented Jul 24, 2021

Experimentally FOO.mp4 can get renamed either to FOO_mp4.MP4 or
FOO.MP4. It's unclear what triggers this behavior (presumably
something about the video content, maybe something that triggers a
re-encode or re-mux), but this avoids duplicating the video on every
smugcli sync invocation for files that get renamed like this.

Fixes #21. (As far as I can tell.)

Experimentally FOO.mp4 can get renamed either to FOO_mp4.MP4 or
FOO.MP4. It's unclear what triggers this behavior (presumably
something about the video content, maybe something that triggers a
re-encode or re-mux), but this avoids duplicating the video on every
`smugcli sync` invocation for files that get renamed like this.

Fixes graveljp#21. (As far as I can tell.)
@firecat53
Copy link

Works great for mp4. This probably should be extended to include the other supported Smugmug video formats. Thanks!

@durin42
Copy link
Contributor Author

durin42 commented Jul 28, 2021

Of the other formats, I only have any .mov files handy, and they seem to consistently not trigger this behavior.

@durin42
Copy link
Contributor Author

durin42 commented Jul 28, 2021

(Obviously evidence of other behavior would be helpful - if all movies get the same behavior on upload, that's great and we can fix this PR easily, but probably better to do as a follow-up for specific behaviors as they're observed?)

@firecat53
Copy link

So far, it also doesn't seem to work for .avi or .m4v

@durin42
Copy link
Contributor Author

durin42 commented Jul 28, 2021 via email

@firecat53
Copy link

Sorry, we're just heading out. I'm just seeing that non-mp4 file types are not getting uploaded at all.

@firecat53
Copy link

After playing with this for a couple of days here's what I see:

  1. This PR does fix most issues with MP4 files.
  2. Additional supported video formats need to be added to DEFAULT_MEDIA_EXT and VIDEO_EXT (avi, m4v, m4a, mpeg, mpg). Probably not in this PR though.
  3. There's an issue with the video file syncing based on the modification times. The threshold is 1 second between modifications times, but I have a lot of files that have been untouched for years and continually get deleted and reuploaded because there's a >1 sec difference between the metadata values grabbed for modification times. There's always that difference, no matter how many times it gets re-uploaded.
  4. hachoir-metadata is detecting the modification date as 1946 for a number of my video files that have a 2012 date. Other tools like exiftool correctly extract the modification date. Hachoir also is unmaintained and should probably be replaced by another library such as ffmpeg-python.

Sorry to dump all this here! I'll open issues for the other items.

Thanks,
Scott

@graveljp
Copy link
Owner

Hi!

Thanks for the contributions. And sorry for the radio silence.

Could it be that SmugMug fixed the "bug" where they were putting the extension in the filename (e.g. file_mp4.MP4)? I was trying to fix MP4 sync months ago before getting into the rabbit hole of mismatching timestamps. But back then, all uploaded mp4 were renamed to "file_mp4.MP4". Using the very same sample files today, I can no longer observe this behavior, files are now named as you'd expect: "file.MP4".

The same thing happened with HEIC files. To support HEIC, I had to puts the extension in the filename, but as of today, the HEIC unit test no longer works and SmugMug no longer seem to be putting "_heic" in the filename using the very same samples.

Maybe, there's value in keeping the + '_' + file_extension[1:] hack to support users who uploaded the files with the old SmugMug, allowing them to sync today without having duplicate files.

On a different note, please try to include unit tests with your pull requests. There's a pretty neat tests/end_to_end_test.py that's super fun to use ;) Unit tests are vital to keep SmugCLI stable because:

  • Any code change could break existing features.
  • SmugMug changes behavior from time to time.
  • If you fix something specific to your workflow, I have no other way of maintaining that feature stable going forward, especially if it involves a file format I don't have.

That being said, I already have a unit test ready to test this pull request, including a very small sample "mp4" file, so don't waste time on that one.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Video file names are altered during upload
3 participants