search

MediaArea / RAWcooked

Encodes RAW audio-visual data into the Matroska container (MKV), using the video codec FFV1 for the image and audio codec FLAC for the sound.
https://mediaarea.net/RAWcooked
BSD 2-Clause "Simplified" License
40 stars 9 forks source link

rgb48le DPX source stored as yuv444p16le #120

Open pjotrek-b opened 6 years ago

pjotrek-b commented 6 years ago

When encoding (ffmpeg-generated) rgb48le DPX files, the resulting FFV1/MKV file is stored as "yuv444p16le". Also tested with rgb48be file (Source: DaVinci/@retokromer).

Is this behavior intentionally?

Additionally, rawcooked outputs the following message during encoding:

Untested FFV1-SLICE-SliceContent:1, please contact info@mediaarea.net if you want support of such file

ffprobe outputs (sorry, not uncut :wink:):

Version: RAWcooked 18.06.20180630

pjotrek-b commented 6 years ago

Just occurred to me: Maybe it's because of the older ffmpeg-libs on my system? Just saw I'm getting a warning in rawcooked's ffmpeg output:

WARNING: library configuration mismatch

rgb48 support in FFV1 might still be marked experimental in those. Will update ffmpeg libs...

JeromeMartinez commented 6 years ago

I tested RGB48LE with this file, is this one fine for you or not?

Additionally, rawcooked outputs the following message during encoding:

FFV1-SLICE-SliceContent is during decoding, did you want to mean decoding instead of encoding? Whatever is the source issue about yuv444p16le, this error is a problem because it means that the decoder can not decode correctly 1 FFV1 frame. Please provide exact command for creating the DPX file, and exact FFmpeg version used.

Anyway, if FFmpeg converts to yuv444p16le, I guess it is due to old FFmpeg, and it breaks the reversibility without warning, this is a concern. I guess I need to find a way to forbid conversion if FFmpeg version is too old (maybe only for 16-bit content).

retokromer commented 6 years ago

RAWcooked might check the version of FFmpeg (or just of its library).

pjotrek-b commented 6 years ago

Tested with updated ffmpeg package now. Old one (which falls back to yuv444p16le): ffmpeg version 2.8.14-0ubuntu0.16.04.1 New(er) one (from PPA): ffmpeg version 3.4.2-1~16.04.york0.2

2nd one now throws the correct warning (for its version):

[ffv1 @ 0x5601133a28c0] 16bit RGB is experimental and under development, only use it for experiments

So, this is not an error - it's just the behavior when using older ffmpeg libs :grin:

Would it be possible to output a warning if pix_fmt cannot be kept by RAWcooked? Because current behavior might create silently-faulty files (e.g. on LTS distro versions)

retokromer commented 6 years ago

In my personal opinion, RAWcooked should check that, when FFmpeg is used, then a library without -strict -2 is used.

JeromeMartinez commented 6 years ago

Would it be possible to output a warning if pix_fmt cannot be kept by RAWcooked?

It is worse, reversibility is not possible (RGB48 to YUV444 is lossy IIRC), so I must block such conversion. I relied on FFmpeg rejecting the command if a pix_fmt is not supported, unfortunately there is a conversion here in case of RGB48 when RGB48 is not supported by the encoder.

Thinking to change behavior in case of RGB48 (the only one having this issue IIRC) by checking FFmpeg version (a bit slower, but just few ms), and depending on the version, either add "-strict -2" (previous behavior during development) or reject the command.

retokromer commented 6 years ago

We cannot reject RGB48! All the work done since two years is precisely to allow it…

JeromeMartinez commented 6 years ago

We cannot reject RGB48! All the work done since two years is precisely to allow it…

Misunderstanding: "depending on the version". If FFmpeg version supports RGB48, no issue! We talk here about when FFmpeg version is old (which is not an issue for people not using RGB48, so I see no reason to mandate newest version of FFmpeg for everyone, i.e. I prefer to have an easy install for most people who prefer to use the FFmpeg version from their distro, and just say to install a new version for people needing it). For Windows and Mac users (without brew), I guess the easier way is to include FFmpeg in the ZIP package.

pjotrek-b commented 6 years ago

Thanks to "--bin-name" (see issue #121), I've now encoded the rgb48le file.

However, I'm still noticing something I wanted to ask if you are aware of that:

Encoding shows correctly "rgb48le", but when I check the resulting file it says "gbrp16le". This is NOT a rawcooked issue, but ffmpeg behavior.

JeromeMartinez commented 6 years ago

However, I'm still noticing something I wanted to ask if you are aware of that: [...]

This is a choice from FFmpeg developers:

Selecting packed or planar output pix_fmt is an arbitrary choice from FFmpeg developers (may just impact performance if a conversion is needed), not impacting how lossless is the content

correctly

both rgb48le and gbrp16le are correct, they both store the same content (RGB 16-bit per component), just stored in a different way (packed vs planar).

pjotrek-b commented 6 years ago

Thank you very much for this thorough and clear explanation! :grinning: Guess we can close this issue?

JeromeMartinez commented 6 years ago

Guess we can close this issue?

I understand the initial issue is still there: I need to forbid conversion to yuv444p16 when the version of FFmpeg is too old (I don't want to force people to update their FFmpeg without needing to do it, i.e. when they don't use RGB48).

pjotrek-b commented 6 years ago

True. What about something like ffmpeg's "-pix_fmt +": If the original pix_fmt cannot be preserved, inform the user about it - with option to force-override it. Avoids silent conversion being overlooked.

Would that be doable? :thinking:

JeromeMartinez commented 6 years ago

If the original pix_fmt cannot be preserved, inform the user about it

But... why?

Not sure I get the issue "pix_fmt":

So the only thing to do is to reject RGB48 when FFmpeg version is too old for supporting it. If user wants to force-override it, FFmpeg can be used directly with the same result.

pjotrek-b commented 6 years ago

I think we're talking past each other :wink: I understood the rgb48 vs gbrp16 after you explained it. Thanks again, btw! :smile:

My suggestion was not to implement a specific "RGB48+FFmpeg-versionX" handling, but was thinking about a generic way of preventing silent pix_fmt conversions in the future? rgb48le to yuv444p16le is just one example for this.

Therefore I suggested a behavior similar to "-pix_fmt +".

But I overlooked that due to calling the FFmpeg binary, RAWcooked might not "see" that FFmpeg converts to a different pix_fmt. Is that the case?

Sorry for maybe overcomplicating things. But silent format changes scare me :scream: :wink: