justdan96 / tsMuxer

tsMuxer is a transport stream muxer for remuxing/muxing elementary streams, EVO/VOB/MPG, MKV/MKA, MP4/MOV, TS, M2TS to TS to M2TS. Supported video codecs H.264/AVC, H.265/HEVC, VC-1, MPEG2. Supported audio codecs AAC, AC3 / E-AC3(DD+), DTS/ DTS-HD.
Apache License 2.0
860 stars 144 forks source link

ac3+XXX file extensions seems a bad idea #802

Open Selur opened 11 months ago

Selur commented 11 months ago

Using ac3+xxx as file extension breaks the possibility to know the output name from knowing the input and the meta file code. :( Why was this changed? I can live with it, but I'm wondering what was the idea behind this? It seems like a bad idea to me.

jcdr428 commented 10 months ago

Hi @Selur, what do you mean by "possibility to know the output name from knowing the input and the meta file code" ? eac3to already produces thd+ac3 file extensions. The change from ec3 to ac3+ec3 came from issue #676.

Selur commented 10 months ago

Hi :) eac3to already produces thd+ac3 file extensions. okay,... in #676 the complaint was that .ec3 was used for e-ac3 even if it got an .ac3 core. And the result was that .ac3+ec3 was added, which is 'okay', but the problem - for me - is the metadata<>extension solution atm. (not really a problem, since I can work around it, when using tsMuxeR to predict the extension, but it seems like 'bad design')

Names of codecs in the meta file:
- V_MPEGI/ISO/VVC   H.266/VVC
- V_MPEGH/ISO/HEVC  H.265/HEVC
- V_MPEG4/ISO/AVC   H.264/AVC
- V_MPEG4/ISO/MVC   H.264/MVC
- V_MS/VFW/WVC1     VC1
- V_MPEG-2          MPEG2
- A_AC3             AC3/AC3+/TRUE-HD
- A_AAC             AAC
- A_DTS             DTS/DTS-Express/DTS-HD
- A_MP3             MPEG audio layer 1/2/3 
- A_LPCM            raw pcm data or PCM WAV file
- S_HDMV/PGS        Presentation graphic stream (BD subtitle format)
- S_TEXT/UTF8       SRT subtitle format.  Encoding MUST be  UTF-8/UTF-16/UTF-32

I think, the output extension should be predictable from the codec metadata. Which is no longer the case. eac3 steams are extracted with A_AC3 truehd streams are extracted A_MLP You can end up with .thd, .ac3+thd, .ec3, .ac3+ec3 or .ac3 extension while using these two tags (A_AC3, A_MLP). Using the same code 'tags' for different outputs seems a bad idea. It seems way more straight forward, to keep with one extension per codec. Also, it seems inconsistent to do this, but use .dts for DTS and DTS-HD audio. (with A_MP3 you can end up with mpa or mp3 as extension,..) a. this seems like this should be documented in the help b. ideally it should be somewhat consistent

Like I wrote, it's not really a problem for me since I can work around it, so no need to change anything, but I would suggest to think over the design of this. :)

jcdr428 commented 10 months ago

@Selur tsMuxer 2.6.12 was not dealing with "pure" eac3 and thd: I included these developments in the open source 2.6.16, without realizing that other softwares such as Hybrid using tsMuxer can also use the codec names.

If we want to be consitent with CodecTags used in ffmpeg and matroska , then we should add A_EAC3, A_MLP and A_TRUEHD, and update the documentation.

Edit: and A_MP3 could be changed to A_MPEG/L3 (or L1 L2), and A_LPCM to A_PCM/XXX/XXX

Selur commented 10 months ago

If we want to be consitent with CodecTags used in ffmpeg and matroska https://ffmpeg.org/doxygen/0.6/matroska_8c-source.html, then we should add A_EAC3, A_MLP and A_TRUEHD, and update the documentation.

I agree, staying consistent with ffmpeg is probably a good idea. For Hybrid and other tools, it should not be much of an issue if there is some documentation. (I adjusted my current dev version to handle the way the current tsMuxeR works.) Just thought I bring this up, since I thought that is 'looked wrong' and could lead to further problems and unnecessary confusion. :)