Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[bug][ugoira] calculate_framerate #4421

Closed
AlttiRi opened this issue Aug 13, 2023 · 13 comments
Closed

[bug][ugoira] calculate_framerate #4421

AlttiRi opened this issue Aug 13, 2023 · 13 comments

Comments

@AlttiRi
Copy link

AlttiRi commented Aug 13, 2023

calculate_framerate returns 1000 fps if uniform is False.

For example, when frames array looks like this:

[
{'file': '000000.jpg', 'delay': 41},
{'file': '000001.jpg', 'delay': 41},
{'file': '000002.jpg', 'delay': 41},
{'file': '000003.jpg', 'delay': 41},

"...",

{'file': '000146.jpg', 'delay': 41},
{'file': '000147.jpg', 'delay': 41},
{'file': '000148.jpg', 'delay': 41},
{'file': '000149.jpg', 'delay': 2000}
]

calculate_framerate returns (None, '1000/1') and ffmpeg uses 1000/1 as -r argument, that results in a corrupted video file (also encoding is taking a lot of time).

['ffmpeg', '-f', 'concat', '-i', 'C:\\.temp\\tmp5s82a966/ffconcat.txt', '-r', '1000/1', '-c:v', 'libvpx', ...

ffconcat.txt contains the follow content:

ffconcat version 1.0
file '000000.jpg'
duration 0.041
file '000001.jpg'
duration 0.041
file '000002.jpg'
duration 0.041
file '000003.jpg'

...

file '000147.jpg'
duration 0.041
file '000148.jpg'
duration 0.041
file '000149.jpg'
duration 2.0
file '000149.jpg'

It seems, omitting -r should help.

ffmpeg.exe -f concat -i ffconcat.txt -c:v libvpx -b:v 5000k -crf 4 out.webm

generates the more correct file, with the last frame is displaying for 2 seconds.

@AlttiRi
Copy link
Author

AlttiRi commented Aug 13, 2023

I currently just replaced


with

if rate_out and rate_out != "1000/1":

to fix it.

@AlttiRi
Copy link
Author

AlttiRi commented Aug 13, 2023

BTW, I recommend to use "-hide_banner", "-loglevel", "error" ffmpeg's arguments to make the gallery-dl output cleaner.

{
    "name": "ugoira",
    "ffmpeg-args": ["-c:v", "libvpx", "-crf", "4", "-b:v", "5000k", "-hide_banner", "-loglevel", "error"],
    "ffmpeg-twopass": true,
    "keep-files": false,
    "mtime": true
}

@AlttiRi
Copy link
Author

AlttiRi commented Aug 18, 2023

I think, the more correct fix is to delete _delay_gcd that calculates Greatest common divisor, since gallery-dl uses ffconcat.txt file that supports non-uniform frame time and return (None, None) from calculate_framerate at the last line.

def calculate_framerate(self, frames):
uniform = self._delay_is_uniform(frames)
if uniform:
return ("1000/{}".format(frames[0]["delay"]), None)
return (None, "1000/{}".format(self._delay_gcd(frames)))
@staticmethod
def _delay_gcd(frames):
result = frames[0]["delay"]
for f in frames:
result = gcd(result, f["delay"])
return result

gcd(41, 2000) is 11000/1 — 1000 FPS that results in a generation of a corrupted file.

@mikf
Copy link
Owner

mikf commented Aug 18, 2023

since gallery-dl uses ffconcat.txt file that supports non-uniform frame time

Last time I checked, FFmpeg assumes an implicit 25 fps when no framerate is specified and it uses the concat demuxer. Any frames that don't fit into this 25 fps window get dropped or duplicated and it produces horribly wrong frame timestamps.

edit: https://trac.ffmpeg.org/ticket/6128

The code should probably enforce an upper fps limit (60? 120?) and ignore frame delay values that differ from the rest (duration 2.0 when everything else is 0.041).


I would generally recommend adding mkvmerge to your PATH when dealing with ugoira and converting to webm/mkv, as that will produce correct timestamps even for non-uniform delays.

@AlttiRi
Copy link
Author

AlttiRi commented Aug 18, 2023

Hm, I tested it manually now, I need to duplicate the last frame two times to make ffmpeg to work properly.

Although, it worked as expected when I use gallery-dl.


ffmpeg -f concat -i ./ffconcat.txt -c:v libvpx -crf 4 -b:v 5000k -y o.webm

ffconcat.txt

ffconcat version 1.0
file '1.jpg'
duration 2.000
file '2.jpg'
duration 2.000
file '3.jpg'
duration 2.000
file '3.jpg'
duration 2.000

In this case 3.jpg is not displayed at all:

ffconcat version 1.0
file '1.jpg'
duration 2.000
file '2.jpg'
duration 2.000
file '3.jpg'
duration 2.000

The result is a 2 seconds video.

@AlttiRi
Copy link
Author

AlttiRi commented Aug 18, 2023

Although, it worked as expected when I use gallery-dl.

It's strange, but it's.


For example, by default, for this https://www.pixiv.net/en/artworks/110956550 gallery-dl creates a file that looks correct, but it's 4 times bigger, since it uses 100 FPS.

In this case frames array contains 16 'delay': 80 and 8 'delay': 90:

[
{'file': '000000.jpg', 'delay': 80},
{'file': '000001.jpg', 'delay': 80},
{'file': '000002.jpg', 'delay': 90},
...
]

GCD is 10. 1000/10 = 100 FPS.

With return (None, None) it creates the correct file with 12.5 FPS.


https://www.pixiv.net/en/artworks/110935156 is similar, also 100 FPS.


Here is a uniform video: https://www.pixiv.net/en/artworks/110894780 ('delay': 33 is for all frames)

Let's make it non-uniform for a test, just add

frames[-1]["delay"] = 2000

as the first line in alculate_framerate function.

Now uniform is False and self._delay_gcd(frames) returns 1. 1000 FPS case.

Encoding takes a lot of time and computing resources. And the result is a corrupted file.

With the suggested fix — returning (None, None) at the last line in calculate_framerate it works fine. The last frame is displaying for 2 seconds.


^ my ugoira config

@mikf
Copy link
Owner

mikf commented Aug 19, 2023

With the suggested fix — returning (None, None) at the last line in calculate_framerate it works fine.

Tested this, yet again (*), and it still shows the same unwanted behavior: Dropped frames, wrong frame timestamps. https://www.pixiv.net/en/artworks/110912034, with 10ms and 20ms frame durations and 360 frames in total, results in a video with only 152 frames and the frame timestamps are all 40ms apart, i.e. 25fps.

Although, it worked as expected when I use gallery-dl.

gallery-dl repeats the last frame by default for this exact reason. (ugoira.repeat-last-frame)

if self.repeat:
append("file '{}'".format(frame["file"]))

(*) I've spent many, many hours over the last couple of years trying to find a general solution and to get FFmpeg to just apply the frame timestamps as given in an ffconcat.txt file, to no avail. To this days I still haven't found anything that works except mkvmerge.

@mikf
Copy link
Owner

mikf commented Aug 20, 2023

I had forgotten about this, but it is already possible to skip the calculate_framerate/GCD step and force a return (None, None): set ugoira.framerate to false/null/"".

@AlttiRi
Copy link
Author

AlttiRi commented Aug 20, 2023

However, it will affect on the cases when uniform is True too. In this case it makes no sense to ignore the frame time from json.

UPD:
However, if it also uses ffconcat.txt it should be OK.

UPD2:
Possibly, it's better to set it anyway, just in case. For the correct FPS displaying when you check the video with external tools.

mikf added a commit that referenced this issue Aug 21, 2023
- when setting this option to a string value,
  pass -hide-banner and -loglevel to FFmpeg
- change default to "error"
mikf added a commit that referenced this issue Aug 21, 2023
only return an output frame rate for non-uniform ugoira
when the frame delay gcd is >= 10, i.e. 100 fps
@mikf
Copy link
Owner

mikf commented Aug 21, 2023

  • The 1/1000 frame rate thing is "fixed" by limiting it to 10/1000/100fps max. Anything over 100fps gets (None, None). (2a3acd3)
  • "-hide_banner", "-loglevel", "error" now get used by default (70bdf32)
  • Setting "framerate" to "uniform" will only apply -r for ugoira with uniform frame delays. Non-uniform ones get (None, None). (c1c73c0)

However, if it also uses ffconcat.txt it should be OK.

Well, no. Using ffconcat.txt without explicit frame rate still defaults to 25fps.

@mikf mikf closed this as completed Aug 21, 2023
@AlttiRi
Copy link
Author

AlttiRi commented Aug 22, 2023

Looks good.


By the way, a new related to Pixiv's "bug" is that it now does not return caption (description) for some* artworks with L0licon (and similar) tag(s).

In the Android application, and therefore in gallery-dl too.


SFW examples of the recently posted artworks:

  • https://www.pixiv.net/en/artworks/110776223
  • https://www.pixiv.net/en/artworks/111030190

image

There is the caption on the site (on desktop), but not in the application.

*I don't even understand the logic when it happens. Some arts have "normal" caption, someones do not. The linked artworks are even non R-18, but they are "shadow banned" (?).

I assume, it's not possible to do something with the Android application's API to "fix" it.

So, just a note.

@mikf
Copy link
Owner

mikf commented Aug 23, 2023

Probably related to #4327

@AlttiRi
Copy link
Author

AlttiRi commented Aug 23, 2023

Probably related to #4327

Ah, yeah, I see, there is also is entirely shadow banned images, they are hidden for Android API when you check user's profile.
/v1/user/illusts?user_id= endpoint does not return any information about them. So, gallery-dl can't even say that there is such images when you download a profile url https://www.pixiv.net/en/users/123456.
They only can be detected in people's bookmarks in the app, since they have a dummy thumbnail with "This work cannot be displayed" text in Japanese. Do not confuse with "Deleted or private".

Shadow banned ("This work cannot be displayed"):
shadow banned
"Deleted or private":
deleted or private

My examples above have only shadow banned caption. So, they are "soft shadow banned".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants