AcademySoftwareFoundation / OpenRV

Open source version of RV, the Sci-Tech award-winning media review and playback software.
Other
589 stars 146 forks source link

[Bug]: OpenRV 2.1.0 getting wrong framerate for movie files #600

Open mircotornow opened 1 month ago

mircotornow commented 1 month ago

What happened?

Hi all.

We noticed a very strange behaviour with our newest rv build (2.1.0 Head: f9e45c7)

When opening movie files (.mov, codec doesn't seem to matter, happens in prores, h264, dnxhd) rv is somehow getting a wrong framerate. (2 instead of 24)

It's also printing following to the console: WARNING: Using non-standard frame rate 2400/1000

It was working fine in previous versions (openrv1.0 and openrv2.0 (our latest working head: f3066e7)

Here openrv2.0 on the left and 2.1.0 on the right image

The FPS is set to 24 for both.

The different framerate results in wrong frame numbers, which is very annoying and makes it unusable in production.

Can anyone confirm that this is happening for them too?

Cheers

List all the operating systems versions where this is happening

CentOS7, Windows10

On what computer hardware is this happening?

any

Relevant console log output

WARNING: Using non-standard frame rate 2400/1000
WARNING: Using non-standard frame rate 2400/1000
WARNING: Using non-standard frame rate 2400/1000
...

..[when playing]..
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred
...

Environment variables

No response

Extra information

No response

Code of Conduct

markreidvfx commented 1 month ago

I have a fix for this issue, I just haven't had time to report or submit a PR. The issue is due to ffmpeg version update. For mov containers, using the streams time_base as the frame rate for timecode is not correct. https://github.com/AcademySoftwareFoundation/OpenRV/blob/6226c5ac2caa3c85599c9c4d50a3d4f15da5bd3b/src/lib/image/MovieFFMpeg/MovieFFMpeg.cpp#L1341

Using AVStream.avg_frame_rate should be the correct thing to use as that is what ffmpeg is using to encode the timecode for mov container files.

https://github.com/FFmpeg/FFmpeg/blob/2a6f84718b172cd0858316281cbbd967f35767f0/libavformat/mov.c#L9503

This issue can be easily replicated using the Alab trailer media. The start frames are incorrect for the mov media in OpenRV.

https://dpel.aswf.io/alab-trailer/

mircotornow commented 1 month ago

Hey @markreidvfx

Thanks for the pointer in the right direction! putting:

AVRational tcRate = tsStream->avg_frame_rate;

did the trick and OpenRV now is getting the correct framerate for mov containers.

However we still get the console log: WARNING: Using non-standard frame rate 2400/1000 (When initially loading a file)

this is coming from ffmpeg: https://github.com/FFmpeg/FFmpeg/blob/7151081e33425010a53c7573e84b274c68ad8087/libavutil/timecode.c#L204

which is called here in openrv: https://github.com/AcademySoftwareFoundation/OpenRV/blob/e551a0d04fdf893d55b6aeb9de515e61cf149165/src/lib/image/MovieFFMpeg/MovieFFMpeg.cpp#L1364

and ALSA lib pcm.c:8424:(snd_pcm_recover) underrun occurred (when playing a .mov source)

So it seems it's calculating the framerate wrong somewhere else too.

markreidvfx commented 1 month ago

I believe it needs to be fixed in two spots. here too. https://github.com/AcademySoftwareFoundation/OpenRV/blob/6226c5ac2caa3c85599c9c4d50a3d4f15da5bd3b/src/lib/image/MovieFFMpeg/MovieFFMpeg.cpp#L3611

Maybe that will fix your audio issue.

I also removed those weird hack about wrong framerates here

https://github.com/AcademySoftwareFoundation/OpenRV/blob/6226c5ac2caa3c85599c9c4d50a3d4f15da5bd3b/src/lib/image/MovieFFMpeg/MovieFFMpeg.cpp#L3615

I'm not really sure what that is all about, it seems very incorrect to me.

I also only limited using avg_frame_rate to mov container files by checking file format in the AVFormatContext. Using avg_frame_rate will lead to incorrect start frames mxf files, this again can be verified with the Alab trailer mxf media.

mircotornow commented 1 month ago

for mxf you are right, but I think mp4 needs the same adjustment as mov

have you tested with mp4 files?

markreidvfx commented 1 month ago

MOV and MP4 are essentially the same format, they use the same demuxer in ffmpeg. It's used for all these formats. mov,mp4,m4a,3gp,3g2,mj2,psp,m4b,ism,ismv,isma,f4v,avif,heic,heif

(It's also used for cr3 raw files)

https://github.com/FFmpeg/FFmpeg/blob/a7449e4cbb36a65b0d3084a4d0ee66c880a4b4e1/libavformat/mov.c#L11018

AVFormatContext->iformat contains a list, so I'm currently just checking if the list contains mov and it should work for all of them. I could also be using the iformat class_name, I'd need to check

markreidvfx commented 1 month ago

MKV and other containers are another issue entirely... If there is no metadata telling us what the timecode rate its impossible to know the start_frame exactly. moov Atom (mov) and MXF formats actually store the timecode as the number of frames, not a string, it would be nicer if ffmpeg exposed that, or the tc rate.

mircotornow commented 1 month ago

how are you exactly checking if the list contains mov?

Before I was thinking about checking it like isMP4format() and adapted it from there but I'm pretty sure this would only work for .mov ? (untested so far)

https://github.com/mircotornow/OpenRV/commit/cb276121d6497b0e00eb70728d6da1fed7e7bba1#diff-f87f8f114f0b9c97fdc4c1c6b87698cf860d0e01d0ae5bafdd55bfa24dffcfc7R706

markreidvfx commented 2 weeks ago

I'm currently doing the same thing. strstr(avFormatContext->iformat->name, "mov")