Doc: Fixes within FFmpeg plugin docs #1097
Conversation
Fix: Header says The MTL Plugin for FFmpeg, instead of The Ffmpeg Plugin for MTL Add: Note about --prefix option to build in a different directory than default Fix: All FFmpeg capitalization issues Fix: Added space to ST 2110 and dash to 10-bit Fix: H.265 and HEVC being mentioned twice, while it is the same video compression standard Fix: SVT-JPEGXS -> SVT-JPEG-XS (as this is the wording present in the repository) Fix: Spaces added where required, other punctuation
MateuszGrabuszynski
requested review from
DawidWesierski4,
Sakoram,
skolelis and
Falron98
as code owners
ecosystem/ffmpeg_plugin/README.md
Outdated
|
|
||
| ```bash | ||
| ffmpeg -p_port 0000:af:01.0 -p_sip 192.168.96.2 -p_rx_ip 239.168.85.20 -udp_port 20000 -payload_type 112 -fps 59.94 -video_size 1920x1080 -st22_codec h264 -f mtl_st22 -i "k" -f rawvideo /dev/null -y | ||
| ``` | ||
|
|
||
| ### 3.3. SVT-JPEGXS | ||
| ### 3.3. SVT-JPEG-XS |
There was a problem hiding this comment.
This was even pointed in the repository of JPEG XS:
https://github.com/OpenVisualCloud/SVT-JPEG-XS/issues/1 "JPEG XS has no hyphen"
Every instance where we are talking about JPEG XS the hyphen should be removed.
p.s. Not sure if in JPEG XS repository this was corrected.
There was a problem hiding this comment.
Thank you for pointing that out! I saw the issue mentioned and totally agree not to use JPEG-XS, but JPEG XS.
I now see that SVT-JPEG-XS is used only in the repository name, but the description does not contain the hyphens in the project name (is using SVT JPEG XS).
Will correct shortly, including SVT HEVC to keep it consistent.
ecosystem/ffmpeg_plugin/README.md
Outdated
| Reading from a yuv stream from local source file, encode with h264 codec and sending a ST 2110-22 codestream on 239.168.85.20:20000 with payload_type 112: | ||
|
|
||
| ```bash | ||
| ffmpeg -stream_loop -1 -video_size 1920x1080 -f rawvideo -pix_fmt yuv420p -i yuv420p_1080p.yuv -filter:v fps=59.94 -c:v libopenh264 -p_port 0000:af:01.1 -p_sip 192.168.96.3 -p_tx_ip 239.168.85.20 -udp_port 20000 -payload_type 112 -f mtl_st22 - |
There was a problem hiding this comment.
Out of the scope of this PR:
@DawidWesierski4 @MateuszGrabuszynski We should not probably reuse the same payload number 112 for ST 2110-22 if we use it as a payload type for ST 2110-20 as this can bring confusion to end user in my opinion. I would not even mention that the number was chosen randomly from a small range of possibilities at some point of development, but it is not industry-wide used one.
This may look like non-impactful short int thing, but can bring lots of not so obvious and easy to debug issues. What we brought from joint work with Piotr on NAB2024 presentation is that the most commonly used values are:
96- video (ST 2110-20)97- audio98- compressed video (ST 2110-22)
but most impotent is the distinction itself, a paragraph form first site found regarding this:
payload_ type (PT)
The value of payload type is selected from the range 96-127. It is recommended to avoid numbers frequently used for audio (97) and video (96), and for example use 104 for DICOM Metadata Essence. The value shall be associated to the media type "application" and the subtype "dicom" in the SDP. E.g., (DICOM Metadata on port 12345).
https://dicom.nema.org/medical/dicom/current/output/chtml/part22/sect_6.2.html
There was a problem hiding this comment.
Totally agree. Have not noticed that it is repeatedly the same number. :|
I was once told the payload_type values are hardcoded here (in MTL) and since then, I am unsure how to handle that topic. I should probably check myself to finally confirm or deny that.
My opinion is that we should probably use the industry standard values (as mentioned by you) to avoid confusion. If the hardcoded values statement is false, I will fix it here.
There was a problem hiding this comment.
As it seems the "hardcoded" values are actually just defaults, I aligned the docs with numbers provided by you, and added notes about the defaults. Please check whether that is any better. :)
Fix: Header says The MTL Plugin for FFmpeg, instead of The Ffmpeg Plugin for MTL
Add: Note about --prefix option to build in a different directory than default
Fix: All FFmpeg capitalization issues
Fix: Added space to ST 2110 and dash to 10-bit
Fix: H.265 and HEVC being mentioned twice, while it is the same video compression standard
Fix: SVT-JPEGXS -> SVT JPEG XS + SVT-HEVC -> SVT HEVC
Fix: Spaces added where required, other punctuation
Fix: payload_type changed from defaults to more common values