Opened 12 years ago

Closed 12 years ago

#1754 closed defect (fixed)

Error in Matroska Muxer

Reported by: Dan 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 by Carl Eugen Hoyos, 12 years ago

Keywords: mkv regression added; MKV Matroska removed
Status: newopen
Version: git-master

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

comment:2 by Michael Niedermayer, 12 years ago

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 by Dan, 12 years ago

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 by reimar, 12 years ago

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 by Michael Niedermayer, 12 years ago

Resolution: fixed
Status: openclosed
Note: See TracTickets for help on using tickets.