Opened 7 years ago

Closed 7 years ago

#1754 closed defect (fixed)

Error in Matroska Muxer

Reported by: Dan203 Owned by:
Priority: important Component: avformat
Version: git-master Keywords: mkv regression
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

On line 635 of matroskaenc.c there is an error. The line is currently...

put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYUNIT, 3);

it should either be deleted completely or set to 0 instead of 3. (0 is default so deleting it should be OK)

According to the MKV specification...

http://matroska.org/technical/specs/index.html

...the DisplayUnit? value should only be set to 3 if the DisplayHeight? and DisplayWidth? values are set to aspect ratio values. However the lines above that one clearly show that DisplayHeight? and DisplayWidth? are being set to pixel values that are adjusted for aspect.

Most players ignore this value, but some hardware players have issues with files created with this incorrect DisplayUnit? value.

Change History (5)

comment:1 Changed 7 years ago by cehoyos

  • Keywords mkv regression added; MKV Matroska removed
  • Status changed from new to open
  • Version set to git-master

Please send a patch to ffmpeg-devel, patches generally receive more attention there.

comment:2 Changed 7 years ago by michael

Please explain which hardware player has a problem with which value pair exactly and how this player misbehaves.
I do not see a discrepancy between the code and the spec, we store a display aspect ratio as case 3 is intended. keep in mind DAR = SAR*w/h. So i need more information here what the problem exactly is

comment:3 Changed 7 years ago by Dan203

I had reports from 3 customers, one using a Popcorn Hour, one using a "Panasonic TV" and another using Media Player Classic. All of them complained that either the file did not play at all or it playing squished. I put out a build where the only change I made was commenting out that one line that sets the DisplayUnit? and all 3 reported it fixed the problem.

Perhaps I'm wrong in my assessment of the spec, but all I know is that when the value set to 3 it fails on at least 3 players and with it set to 0 or not set at all it works in every player I've tried. And when I remux the file with MKVMerge that value is removed, so at the very least it seems to be unnecessary.

comment:4 Changed 7 years ago by reimar

I can see two ways this could go wrong:
1) Because 0 is default those players forgot implementing support for 3
2) Those players expect that an aspect value is reduced, so if we store the width/height without reducing it, it might end up causing overflows in some calculation (seems unlikely?)

comment:5 Changed 7 years ago by michael

  • Resolution set to fixed
  • Status changed from open to closed
Note: See TracTickets for help on using tickets.