kannibalox / PtpUploader

PtpUploader is a bot that automatically uploads releases to PTP.
69 stars 21 forks source link

DVD screenshots aspect ratio is sometimes incorrect #8

Closed newadventure079 closed 2 years ago

newadventure079 commented 2 years ago

If you search for "720x540" in the PTP thread that's linked in the readme here, you'll see 2 posts about the aspect ratio not being correct for DVD images. I've run into the same issue and had to manually edit the code to also fix the issue.

Below are the 2 posts. Has this issue been fixed in PtpUpload 1.0 or does it still exist? After reading these posts again, I do remember editing the code manually to switch to mediainfo instead of ffmpeg to get the resolution.

@TnS I uploaded Baarìa [2009] - DVD9 / VOB IFO / DVD / NTSC and on PTP the VTS_01_0.IFO says "Resolution: 720x480 ~> 853x480" so the screenshots produced by PTPUploader should be 854x480 but instead they were 720x540

The command I ran was:

python ~/PTP/Program/src/PtpUploader/ReleaseInfoMaker.py /volume1/data/LTS/BAARIA/

Output from the log:

[2017-09-21 12:40:01] INFO     Making screenshot with ffmpeg from '/volume1/data/LTS/BAARIA/VIDEO_TS/VTS_01_1.VOB' to '/volume1/data/LTS/00000001.png'.
[2017-09-21 12:40:01] INFO     Pixel aspect ratio wasn't 1:1, scaling video to resolution: '720x540'.

It might be related to ffmpeg outputting "DAR 4:3" but in PTP, VTS_01.0.IFO says "Aspect ratio: 16:9". Maybe mediainfo could be used instead since it outputs "Display aspect ratio : 16:9"

Edit: I edited the code in __CalculateSizeAccordingToAspectRatio() to use mediainfo instead of ffmpeg and it now correctly detects 16:9 and 4:3. Therefore the screenshots were of the correct resolution (720x540 or 854x480) for both 4:3 and 16:9 DVDs
Hi TnS. Firstly I'd like to thank you for maintaining this fine program. However the reason that I'm posting is that I think that I've found a bug in the ffmpeg screenshot function relating to anamorphic video.

In your code below, you always resize the width of screenshots to match the required DAR.
Tool/Ffmpeg.py Line 41 - 50 wrote:
...
        newWidth = ( height * darX ) / darY
        newWidth = int( newWidth )
        if abs( newWidth - width ) <= 1:
            return

        # For FFmpeg frame size must be a multiple of 2.
        if ( newWidth % 2 ) != 0:
            newWidth += 1

        self.ScaleSize = "%sx%s" % ( newWidth, height )
...

The problem with this is that some anamorphic video needs a resized height instead. Otherwise, you end up downscaling the video.

Example movies:
torrents.php?id=65555&torrentid=380608
The Mark of Zorro [1920] - DVD9 / VOB IFO / DVD / NTSC

When the current function is used to take the screenshots, the result is a downscaled 640x480, when the proper DAR is 720x540, keeping in mind that the SAR is 720x480.

To solve this issue, there needs to be a check whether the height or width needs to be resized accordingly.
Tools/Ffmpeg.py Line 41 - 65 wrote:
...
                # Choose whether we resize height or width.
                if float( width ) / height < float( darX ) / darY:
                        # Resize width
                        newWidth = ( height * darX ) / darY
                        newWidth = int( newWidth )
                        if abs( newWidth - width ) <= 1:
                                return

                        # For FFmpeg frame size must be a multiple of 2.
                        if ( newWidth % 2 ) != 0:
                                newWidth += 1

                        self.ScaleSize = "%sx%s" % ( newWidth, height )
                else:
                        # Resize the height
                        newHeight = ( width * darY ) / darX
                        newHeight = int( newHeight )
                        if abs( newHeight - height ) <= 1:
                                return

                        # For FFmpeg frame size must be a multiple of 2.
                        if ( newHeight % 2 ) != 0:
                                newHeight += 1

                        self.ScaleSize = "%sx%s" % ( width, newHeight )
...
newadventure079 commented 2 years ago

I just checked this and it is indeed still broken in the latest build

For a test DVD, IFO says Resolution: 720x480 ~> 853x480

PtpUploader created 720x540 screens, instead of 853x480

Mediainfo:

General
Complete name                            : VIDEO_TS/VTS_01_0.IFO
Format                                   : DVD Video
Format profile                           : Program
File size                                : 58.0 KiB
Duration                                 : 1 h 15 min
Overall bit rate mode                    : Variable
Overall bit rate                         : 104 b/s

Video
ID                                       : 224 (0xE0)
Format                                   : MPEG Video
Format version                           : Version 2
Duration                                 : 1 h 15 min
Bit rate mode                            : Variable
Width                                    : 720 pixels
Height                                   : 480 pixels
Display aspect ratio                     : 16:9
Frame rate                               : 29.970 (29970/1000) FPS
Standard                                 : NTSC
Compression mode                         : Lossy

Audio
ID                                       : 128 (0x80)
Format                                   : AC-3
Format/Info                              : Audio Coding 3
Duration                                 : 1 h 15 min
Channel(s)                               : 2 channels
Sampling rate                            : 48.0 kHz
Compression mode                         : Lossy
Language                                 : English

Menu
Duration                                 : 1 h 15 min
00:00:00.000                             : Chapter 1
00:02:10.500                             : Chapter 2
00:08:36.500                             : Chapter 3
00:13:08.500                             : Chapter 4
00:17:50.500                             : Chapter 5
00:25:49.500                             : Chapter 6
00:28:22.000                             : Chapter 7
00:40:31.000                             : Chapter 8
00:52:38.000                             : Chapter 9
01:03:59.000                             : Chapter 10
01:12:35.500                             : Chapter 11
List (Audio)                             : 0

General
Complete name                            : VIDEO_TS/VTS_01_1.VOB
Format                                   : MPEG-PS
File size                                : 1 024 MiB
Duration                                 : 21 min 45 s
Overall bit rate mode                    : Variable
Overall bit rate                         : 6 578 kb/s

Video
ID                                       : 224 (0xE0)
Format                                   : MPEG Video
Format version                           : Version 2
Format profile                           : Main@Main
Format settings                          : CustomMatrix / BVOP
Format settings, BVOP                    : Yes
Format settings, Matrix                  : Custom
Format settings, GOP                     : M=3, N=15
Format settings, picture structure       : Frame
Duration                                 : 21 min 45 s
Bit rate mode                            : Variable
Bit rate                                 : 6 255 kb/s
Maximum bit rate                         : 7 700 kb/s
Width                                    : 720 pixels
Height                                   : 480 pixels
Display aspect ratio                     : 4:3
Frame rate                               : 29.970 (30000/1001) FPS
Standard                                 : NTSC
Color space                              : YUV
Chroma subsampling                       : 4:2:0
Bit depth                                : 8 bits
Scan type                                : Interlaced
Scan order                               : Bottom Field First
Compression mode                         : Lossy
Bits/(Pixel*Frame)                       : 0.604
Time code of first frame                 : 00:00:00;00
Time code source                         : Group of pictures header
GOP, Open/Closed                         : Closed
Stream size                              : 974 MiB (95%)
Color primaries                          : BT.601 NTSC
Transfer characteristics                 : BT.601
Matrix coefficients                      : BT.601

Audio
ID                                       : 189 (0xBD)-128 (0x80)
Format                                   : AC-3
Format/Info                              : Audio Coding 3
Commercial name                          : Dolby Digital
Muxing mode                              : DVD-Video
Duration                                 : 21 min 45 s
Bit rate mode                            : Constant
Bit rate                                 : 192 kb/s
Channel(s)                               : 2 channels
Channel layout                           : L R
Sampling rate                            : 48.0 kHz
Frame rate                               : 31.250 FPS (1536 SPF)
Compression mode                         : Lossy
Stream size                              : 29.9 MiB (3%)
Service kind                             : Complete Main

Menu
kannibalox commented 2 years ago

Hm that code was brought into the repo way back in 98bd0f8, but that doesn't mean it's not bugging out on other cases. Can you also post the output of ffmpeg -i <path to .ifo>?

newadventure079 commented 2 years ago

Ya, it's been an issue for years. I forget the dates of the posts in the forum but they were from a while ago and no one responded. I dont think DVD disc uploads were a major concern for 99% of people 😂

I forgot this repo existed and had just patched it myself but that code was lost when my seedbox expired.

I'll post the ffmpeg output in a bit

newadventure079 commented 2 years ago

ffmpeg didn't like ffmpeg -i <path to .ifo>

ffmpeg -i /Volumes/data/LTS/Movies/movie/VIDEO_TS/VTS_01_0.IFO
ffmpeg version 4.4.1 Copyright (c) 2000-2021 the FFmpeg developers
  built with Apple clang version 13.0.0 (clang-1300.0.29.3)
  configuration: --prefix=/opt/homebrew/Cellar/ffmpeg/4.4.1_5 --enable-shared --enable-pthreads --enable-version3 --cc=clang --host-cflags= --host-ldflags= --enable-ffplay --enable-gnutls --enable-gpl --enable-libaom --enable-libbluray --enable-libdav1d --enable-libmp3lame --enable-libopus --enable-librav1e --enable-librist --enable-librubberband --enable-libsnappy --enable-libsrt --enable-libtesseract --enable-libtheora --enable-libvidstab --enable-libvmaf --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxml2 --enable-libxvid --enable-lzma --enable-libfontconfig --enable-libfreetype --enable-frei0r --enable-libass --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopenjpeg --enable-libspeex --enable-libsoxr --enable-libzmq --enable-libzimg --disable-libjack --disable-indev=jack --enable-avresample --enable-videotoolbox
  libavutil      56. 70.100 / 56. 70.100
  libavcodec     58.134.100 / 58.134.100
  libavformat    58. 76.100 / 58. 76.100
  libavdevice    58. 13.100 / 58. 13.100
  libavfilter     7.110.100 /  7.110.100
  libavresample   4.  0.  0 /  4.  0.  0
  libswscale      5.  9.100 /  5.  9.100
  libswresample   3.  9.100 /  3.  9.100
  libpostproc    55.  9.100 / 55.  9.100
/Volumes/data/LTS/Movies/movie/VIDEO_TS/VTS_01_0.IFO: Invalid data found when processing input
newadventure079 commented 2 years ago

Here is what PTP has on the details page tho:

Screen Shot 2022-01-15 at 7 46 31 PM

Screen Shot 2022-01-15 at 7 46 44 PM

Screen Shot 2022-01-15 at 7 47 01 PM

newadventure079 commented 2 years ago

I think the issue here is the .IFO has the resolution as 16:9, whereas the .VOB has it as 4:3.

__CalculateSizeAccordingToAspectRatio gets the .VOB filename passed to it, so that's why PtpUploader thinks it's 4:3 and therefore the screens are 720x540

I edited the code and made darX = 16 and darY = 9 and the screens were the correct resolution of 854x480

kannibalox commented 2 years ago

Hm I think I know what needs to happen here but don't have a great way to test it. Would it be possible for you to provide a link to an example torrent?

newadventure079 commented 2 years ago

The people in the thread mentioned Baarìa [2009] - DVD9 / VOB IFO / DVD / NTSC and The Mark of Zorro [1920] - DVD9 / VOB IFO / DVD / NTSC so those would probably work for testing

kannibalox commented 2 years ago

I'll double-check Baaria, but Zorro worked fine for me with the current code, and neither have the mismatched aspect ratios.

newadventure079 commented 2 years ago

I was looking on the forums and I saw a moderator say to always look at what the .IFO reports. So for __CalculateSizeAccordingToAspectRatio to get the .VOB filename passed to it is a bug. I bet if we passed it the .IFO filename, it would work

That would also back up my previous comment of "I think the issue here is the .IFO has the resolution as 16:9, whereas the .VOB has it as 4:3."

newadventure079 commented 2 years ago

btw, have you found a file to test with or should I find you one?

newadventure079 commented 2 years ago

Here is some code that works for me and my test cases. All are producing the correct resolution for the screens.

It uses mediainfo and the .IFO file, instead of ffmpeg and the .VOB file

It'd probably be best to pass in the .IFO file to the function, and also call __ReadMediaInfo instead of replicating that code like I did below. I didn't know how to call __ReadMediaInfo

I suppose this should probably be a function in MediaInfo.py

It could probably use some clean up, but it works

def __CalculateSizeAccordingToAspectRatio(self):
        #Get resolution and pixel aspect ratio from mediainfo.

        #probably can just pass in the .ifo file in addition to the .vob file
        ifoPath = self.InputVideoPath[:-5]+'0.IFO'
        args = [Settings.MediaInfoPath, ifoPath]

        #probably can make a call to __ReadMediaInfo() instead here
        proc = subprocess.run(
            args,
            capture_output=True,
            check=True,
        )
        result: str = proc.stdout.decode("utf-8", "ignore")

        #parse mediainfo output for height, width, darX and darY
        height = width = dar = darX = darY = 0
        for line in result.splitlines():
            match = re.search(r'Width\s+:\s+\d+', line)
            if match:
                width = match.group(0)
            match = re.search(r'Height\s+:\s+\d+', line)
            if match:
                height = match.group(0)
            match = re.search(r'Display aspect ratio\s+:\s+\d+:\d+', line)
            if match:
                dar = match.group(0)

        if width != 0:
            width = int(re.sub(r'Width\s+:', '', width).strip())
        if height != 0:
            height = int(re.sub(r'Height\s+:', '', height).strip())
        if dar != 0:
            dar = re.sub(r'Display aspect ratio\s+:', '', dar).strip()
            split = dar.split(':')
            darX = int(split[0].strip())
            darY = int(split[1].strip())
kannibalox commented 2 years ago

Nice! I haven't been able to find any good test files, but your code made me realize it should be pretty safe to trust the mediainfo in general. Give it a try and let me know if it works as expected.

newadventure079 commented 2 years ago

The resolution is correct for my first test case but I ran into an error:

2022-01-20 00:19:44,077 INFO  PtpUploader.ReleaseDescriptionFormatter Pixel aspect ratio isn't 1:1, scaling video to resolution: '854x480'.
Traceback (most recent call last):
  File "/Users/.venv/ptpuploader/bin/ReleaseInfoMaker", line 8, in <module>
    sys.exit(run())
  File "/Users/.venv/ptpuploader/lib/python3.9/site-packages/PtpUploader/ReleaseInfoMaker.py", line 158, in run
    releaseInfoMaker.MakeReleaseInfo(
  File "/Users/.venv/ptpuploader/lib/python3.9/site-packages/PtpUploader/ReleaseInfoMaker.py", line 125, in MakeReleaseInfo
    self.SaveReleaseDescriptionFile(
  File "/Users/.venv/ptpuploader/lib/python3.9/site-packages/PtpUploader/ReleaseInfoMaker.py", line 85, in SaveReleaseDescriptionFile
    releaseDescriptionFormatter = ReleaseDescriptionFormatter(
  File "/Users/.venv/ptpuploader/lib/python3.9/site-packages/PtpUploader/ReleaseDescriptionFormatter.py", line 56, in __init__
    self.__TakeAndUploadScreenshots()
  File "/Users/.venv/ptpuploader/lib/python3.9/site-packages/PtpUploader/ReleaseDescriptionFormatter.py", line 149, in __TakeAndUploadScreenshots
    ] = screenshotMaker.TakeAndUploadScreenshots(
  File "/Users/.venv/ptpuploader/lib/python3.9/site-packages/PtpUploader/Tool/ScreenshotMaker.py", line 91, in TakeAndUploadScreenshots
    self.__TakeAndUploadScreenshot(
  File "/Users/.venv/ptpuploader/lib/python3.9/site-packages/PtpUploader/Tool/ScreenshotMaker.py", line 67, in __TakeAndUploadScreenshot
    self.InternalScreenshotMaker.MakeScreenshotInPng(timeInSeconds, outputPngPath)
  File "/Users/.venv/ptpuploader/lib/python3.9/site-packages/PtpUploader/Tool/Ffmpeg.py", line 100, in MakeScreenshotInPng
    errorCode = subprocess.call(args)
  File "/opt/homebrew/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 349, in call
    with Popen(*popenargs, **kwargs) as p:
  File "/opt/homebrew/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/opt/homebrew/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 1754, in _execute_child
    self.pid = _posixsubprocess.fork_exec(
TypeError: expected str, bytes or os.PathLike object, not list
kannibalox commented 2 years ago

That issue was pretty easy to fix, unfortunately I think I got too clever for my own good, and found some other edge cases that wouldn't've worked properly. Try the latest version.

newadventure079 commented 2 years ago

It looks great! Thanks a lot!! I'll re-open if I find anymore cases 😅