This repository has been archived by the owner. It is now read-only.

Running mkv through mkvtoolnix adds artefacts to mpeg2 video #2045

Closed
Afullmark opened this Issue Jul 15, 2017 · 15 comments

Comments

2 participants
@Afullmark

Afullmark commented Jul 15, 2017

Version: 13.0.0 on the Mac.
How to reproduce: Add mkv to mkvtoolnix, and, say, uncheck audio, multiplex to new mkv. The resulting mkv produced will have added artefacts to the mpeg2 video that were not in the original.

You will see the issue between 10:18 and 10:21:

Running file through mkvtoolnix results in the following:
10 20 in timeline artefacts added

image taken at 10:20 in timeline

The artefact is not present in the original.
10 20 no atrefacts in original

image taken at 10:20 in timeline

I cannot make a small sample of the original file, obviously, because it will result in the artefact being present. Here are the links to the full files (original, 7.19GB and the one run through mkvtoolnix, 6.85GB); i'll provide a password to you on request to download them:

  1. MKV produced by mkvtoolnix: https://www.dropbox.com/s/p4kg25bvwg4d03v/The%20MKV%20created%20by%20mkvtoolnix%20with%20artefact.mkv?dl=0

2, Original:
https://www.dropbox.com/s/jn6fw6m81qhl6yf/Original%20MKV.mkv?dl=0

I narrowed down to mkvtoolnix being the problem by doing the following:

A

  1. extract m2v from original mkv
  2. add m2v to mkv using mkvtoolnix
  3. Artefacts added to video

B

  1. extract m2v from original mkv
  2. add m2v to mkv using ffmpeg
  3. No artefacts added to video

Any form of running the original through mkvtoolnix will result in the addition of mpeg2 video artefacts.

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 15, 2017

Owner

Thanks for the comprehensive report, but this won't be fixed. The MPEG-1/2 handling code contains some more issues that won't be worked on anymore as those formats are simply outdated, and I lack the time to fix or implement everything I actually want to and more users care about anyway.

You should be able to circumvent this issue by muxing the video to Matroska with a different program (e.g. MakeMKV) and then processing that file with MKVToolNix as that should use other code paths.

Owner

mbunkus commented Jul 15, 2017

Thanks for the comprehensive report, but this won't be fixed. The MPEG-1/2 handling code contains some more issues that won't be worked on anymore as those formats are simply outdated, and I lack the time to fix or implement everything I actually want to and more users care about anyway.

You should be able to circumvent this issue by muxing the video to Matroska with a different program (e.g. MakeMKV) and then processing that file with MKVToolNix as that should use other code paths.

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 15, 2017

Owner

I'll download the sample anyway, just in case I do happen to find myself with some free time & some motivation to debug this.

Additionally: of course I would accept patches that fix the issue; that'd make me very happy. It's just unlikely that I'm going to spend time on this myself.

Owner

mbunkus commented Jul 15, 2017

I'll download the sample anyway, just in case I do happen to find myself with some free time & some motivation to debug this.

Additionally: of course I would accept patches that fix the issue; that'd make me very happy. It's just unlikely that I'm going to spend time on this myself.

@Afullmark

This comment has been minimized.

Show comment
Hide comment
@Afullmark

Afullmark Jul 15, 2017

Afullmark commented Jul 15, 2017

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 15, 2017

Owner

mkvmerge has to determine frame boundaries for containers in which the frame boundaries aren't clear and where timestamps aren't provided for all frames. That includes MPEG program streams, what VOBs on DVDs use.

If it is read from a container which does provide those things, such as Matroska, each frames are taken as-is, and therefore there's no further "meddling" with the bitstream required.

Owner

mbunkus commented Jul 15, 2017

mkvmerge has to determine frame boundaries for containers in which the frame boundaries aren't clear and where timestamps aren't provided for all frames. That includes MPEG program streams, what VOBs on DVDs use.

If it is read from a container which does provide those things, such as Matroska, each frames are taken as-is, and therefore there's no further "meddling" with the bitstream required.

@Afullmark

This comment has been minimized.

Show comment
Hide comment
@Afullmark

Afullmark Jul 15, 2017

Thats what I mean:

Forgetting m2v, I can take the makemkv original MKV and just put it through mkvtoolnix (without changes) but just creating a new mkv and the artefacts are placed into the video stream. Are you saying this shouldn't happen? If so, it is happening.

Or are you saying that the video stream inside the original MKV is damaged and mkvtoolnix is 'trying' to fix it?

Afullmark commented Jul 15, 2017

Thats what I mean:

Forgetting m2v, I can take the makemkv original MKV and just put it through mkvtoolnix (without changes) but just creating a new mkv and the artefacts are placed into the video stream. Are you saying this shouldn't happen? If so, it is happening.

Or are you saying that the video stream inside the original MKV is damaged and mkvtoolnix is 'trying' to fix it?

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 15, 2017

Owner

Are you saying this shouldn't happen? If so, it is happening.

Then I don't know what's going on. And like I said, I'm likely not going to debug this. I will try to reproduce it here, at least, once the downloads's finished.

Owner

mbunkus commented Jul 15, 2017

Are you saying this shouldn't happen? If so, it is happening.

Then I don't know what's going on. And like I said, I'm likely not going to debug this. I will try to reproduce it here, at least, once the downloads's finished.

@Afullmark

This comment has been minimized.

Show comment
Hide comment
@Afullmark

Afullmark Jul 15, 2017

Thank you very much. I do appreciate the time you've spent already chatting with me. I am slightly concerned now that when I have used mkvtoolnix in the past to join mkv files (say, when a film is spread across to discs) containing mpeg2 video this issue might be there also.

Thanks again.

Afullmark commented Jul 15, 2017

Thank you very much. I do appreciate the time you've spent already chatting with me. I am slightly concerned now that when I have used mkvtoolnix in the past to join mkv files (say, when a film is spread across to discs) containing mpeg2 video this issue might be there also.

Thanks again.

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 15, 2017

Owner

You're welcome.

I can reproduce the issue with the sample file you've provided. So I'm all set, should I decide to invest further time in this.

Owner

mbunkus commented Jul 15, 2017

You're welcome.

I can reproduce the issue with the sample file you've provided. So I'm all set, should I decide to invest further time in this.

@mbunkus mbunkus removed the res:wontfix label Jul 15, 2017

@mbunkus mbunkus reopened this Jul 15, 2017

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 15, 2017

Owner

I was slightly curious due to the source file being a Matroska file, and as I've said above in such a case the content shouldn't be modified.

I was remembering that wrong; though. Even for Matroska files one thing is done: the removal of stuffing bytes. Stuffing bytes are superfluous bytes sometimes inserted into the bitstream in order to achieve a minimum bitrate (practical application: so that your DVD or Blu-ray drive keeps on spinning and doesn't have to spin down when the bitrate is low, which would be very annoying for the viewers as spinning up/down is very noticeable when watching a movie).

Such stuffing bytes aren't needed for decoding at all. Inside Matroska such bytes are even more superfluous. Therefore mkvmerge contains code to remove them.

And as soon as I comment that code out the artifacts disappear. So it's a bug in a piece of code where I'm definitely going to investigate because it doesn't just affect MPEG elementary streams but also packaged MPEG frames (the elementary stream parser is the code I don't want to touch anymore, but as I said this bug isn't in there but somewhere else).

Owner

mbunkus commented Jul 15, 2017

I was slightly curious due to the source file being a Matroska file, and as I've said above in such a case the content shouldn't be modified.

I was remembering that wrong; though. Even for Matroska files one thing is done: the removal of stuffing bytes. Stuffing bytes are superfluous bytes sometimes inserted into the bitstream in order to achieve a minimum bitrate (practical application: so that your DVD or Blu-ray drive keeps on spinning and doesn't have to spin down when the bitrate is low, which would be very annoying for the viewers as spinning up/down is very noticeable when watching a movie).

Such stuffing bytes aren't needed for decoding at all. Inside Matroska such bytes are even more superfluous. Therefore mkvmerge contains code to remove them.

And as soon as I comment that code out the artifacts disappear. So it's a bug in a piece of code where I'm definitely going to investigate because it doesn't just affect MPEG elementary streams but also packaged MPEG frames (the elementary stream parser is the code I don't want to touch anymore, but as I said this bug isn't in there but somewhere else).

@Afullmark

This comment has been minimized.

Show comment
Hide comment
@Afullmark

Afullmark Jul 15, 2017

Great news. Totally understand regarding the elementary stream parser code. Hopefully it's narrowed down enough for an un-arduous fix. As you are busy i'll trouble you no more on this; i'll keep an eye on this thread for when you've found a fix.

I use mkvtoolnix all the time. It's great. And again, I appreciate the trouble you've gone to in identifying this problem.

Afullmark commented Jul 15, 2017

Great news. Totally understand regarding the elementary stream parser code. Hopefully it's narrowed down enough for an un-arduous fix. As you are busy i'll trouble you no more on this; i'll keep an eye on this thread for when you've found a fix.

I use mkvtoolnix all the time. It's great. And again, I appreciate the trouble you've gone to in identifying this problem.

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 15, 2017

Owner

Here is a new pre-build DMG for you that includes the fix.

Owner

mbunkus commented Jul 15, 2017

Here is a new pre-build DMG for you that includes the fix.

@Afullmark

This comment has been minimized.

Show comment
Hide comment
@Afullmark

Afullmark Jul 15, 2017

Afullmark commented Jul 15, 2017

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 15, 2017

Owner

You're welcome. And of course it will.

Owner

mbunkus commented Jul 15, 2017

You're welcome. And of course it will.

@Afullmark

This comment has been minimized.

Show comment
Hide comment
@Afullmark

Afullmark Jul 19, 2017

I just tried the pre-build; I had to fix a few (136) mpeg elemental streams that had a Sequence Display Extension that was causing the nvidia shield tv to display videos with the wrong horizontal size: 720x576 wrongly playing as 540x576 and 720x480 wrongly playing as 540x480, for example.

I have remuxed those mpeg elementary streams (m2v) with mkvtoolnix and the artefact issue i reported is gone. Thanks ever so much for this and the pre-build so I could get on with fixing my files.

I'll drop you a donation.

Afullmark commented Jul 19, 2017

I just tried the pre-build; I had to fix a few (136) mpeg elemental streams that had a Sequence Display Extension that was causing the nvidia shield tv to display videos with the wrong horizontal size: 720x576 wrongly playing as 540x576 and 720x480 wrongly playing as 540x480, for example.

I have remuxed those mpeg elementary streams (m2v) with mkvtoolnix and the artefact issue i reported is gone. Thanks ever so much for this and the pre-build so I could get on with fixing my files.

I'll drop you a donation.

@mbunkus

This comment has been minimized.

Show comment
Hide comment
@mbunkus

mbunkus Jul 19, 2017

Owner

That's great to hear (the "it's working part, though a donation is always welcome, too, of course). Thanks for the feedback.

Owner

mbunkus commented Jul 19, 2017

That's great to hear (the "it's working part, though a donation is always welcome, too, of course). Thanks for the feedback.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.