After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 323332 - mpeg2dec sets wrong aspect ratio for 16:9 dvds
mpeg2dec sets wrong aspect ratio for 16:9 dvds
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
git master
Other Linux
: Normal normal
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-12-06 01:48 UTC by j^
Modified: 2008-10-04 21:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch fixing libmpeg2 (474 bytes, patch)
2006-01-13 08:44 UTC, Michal Benes
none Details | Review

Description j^ 2005-12-06 01:48:55 UTC
trying to decode mpeg2-ntsc with 16:9 aspect ratio(non square pixels)
mpeg2dec sets a par of 128:81

      "pixel-aspect-ratio", GST_TYPE_FRACTION, mpeg2dec->pixel_width,
      mpeg2dec->pixel_height,

this is wrong since that way one ends up with a frame aspect ratio of 2.370
32:27 would be the right pixel aspect ratio in this case. 
this bug renders apps like thoggen useles for a common user.
Comment 1 Edward Hervey 2005-12-06 10:39:48 UTC
do you have a sample file we could test this with ?
Comment 2 j^ 2005-12-06 16:03:09 UTC
sample provided off bugzilla, write me if you need a sample.
Comment 3 Edward Hervey 2005-12-06 17:55:43 UTC
The current display aspect ratio is of 720 / 480 * 128 / 81 = 64 / 27. So it's
exactly two times bigger than the good DAR.
The pixel aspect ratio is 'filled' in by the mpeg2dec library and then copied
over by the plugin in the handle_sequence() function. The values returned by
mpeg2dec are:
width, height: 720, 480
picture-width,height: 720, 480
display-width,height: 540, 480
pixel-width,height: 128, 81

Something which might be of interest, is that mpeg2dec is saying that the
sequence is PAL and not NTSC.
Comment 4 Jan Schmidt 2005-12-08 11:33:51 UTC
This is a bug in libmpeg2. Because this is video from a pan-and-scan DVD, it has
a 720x480 video, with an intended 16:9 display ratio, and also in the MPEG data
(in the sequence_display_extension) is information telling the decoder to crop
to 540x480 pixels.

720x480->16:9 display implies (16*480)/(9*720) = 1.185  pixel-aspect-ratio (or
32/27) as we want. libmpeg2 is computing the pixel-aspect-ratio from the CROPPED
window, however (which it should not). A 540x480 window stretched to 16:9
implies (16*480)/(9*540) = 128/81 pixel-aspect-ratio.

We could work around this in mpeg2dec by checking whether the picture-size
matches the display-size, but the correct fix should be done in libmpeg2
Comment 5 Michal Benes 2006-01-13 08:44:54 UTC
Created attachment 57270 [details] [review]
Patch fixing libmpeg2

This is patch for libmpeg2 to fix this issue. I am trying to push this patch to libmpeg2 developers. Does anybody here know some them?
Comment 6 Michal Benes 2006-01-16 09:27:39 UTC
Well, the problem seems to be far more complicated. See
http://sourceforge.net/mailarchive/forum.php?thread_id=9460940&forum_id=729
Comment 7 Jan Schmidt 2006-01-16 12:20:28 UTC
Yep, ok - I agree with the interpretation of the mpeg spec in the email thread (I missed that clause previously).

In terms of DVD vobs, I guess the problem is that Pan & Scan is an optional output mode, and is the only time that the sequence display extension is used. 

As mentioned in the thread, DVDs do have access to other information about the display mode, but this is only (I think?) stored in the .ifo files, not in the .vobs.

I can't see any way to automatically adapt libmpeg2 or mpeg2dec for these streams and still have it work for ones that match the MPEG spec.
Comment 8 Michal Benes 2006-01-16 12:43:11 UTC
This is bad. From all I know, it seems that such specs-violating mpeg streams are quite common and that the authoritative information should be taken from the .ifo file. (BTW: mplayer ignores PAR from mpeg2dec completely)

Now, the question is how to solve this. I see two possibilities: 
- create a dvdmpeg2dec that will try guess the correct PAR knowing that the given stream is from DVD. But I do not know how to handle this by autopluggers (put dvdstream=true to caps?)
- propagate the information from .ifo file to mpeg2dec somehow. Probably using application event - but this assumes that the source is dvdsrc.
Comment 9 Jan Schmidt 2006-01-30 14:35:11 UTC
It may be possible to detect NAV streams in the mpeg demuxer for MPEG-2 streams and add an extra parameter in the caps that the mpeg2dec can use to know when to override the encoded PAR from the stream.
Comment 10 Michal Benes 2006-01-30 15:21:30 UTC
What means "NAV stream"?
It would be good to have the problem solved without using .ifo files.
Comment 11 Jan Schmidt 2006-01-30 16:54:51 UTC
VOB mpeg2 streams have NAV and DSI packets inside one of the private streams.

This is at the mpeg demuxer level, so it would need to pass the information down to the mpeg decoder via caps.
Comment 12 Christian Fredrik Kalager Schaller 2006-06-02 15:47:43 UTC
Michal are you still looking into this issue?
Comment 13 Michal Benes 2006-06-07 10:56:14 UTC
We have solved this issue by overriding the aspect ratio using the value from the .ifo file.

There is a patch that adds DAR parameter to mpeg2dec plugin. But I have not posted it anywhere since this solution needs some application interaction (namely reading DAR prom .ifo and setting it to the mpeg2dec plugin)
Comment 14 Sebastian Dröge (slomo) 2008-05-05 08:48:35 UTC
So is anything still to be done here? Maybe mpeg2dec should prefer pixel aspect ratios given via caps over then ones it finds in the mpeg stream? And then we teach the demuxer (or dvdsrc?) to put it into the caps if possible?
Comment 15 David Schleef 2008-05-06 19:02:39 UTC
It makes more sense for the (mythical) dvdbin element to fix up the p-a-r coming out of mpeg2dec.
Comment 16 David Schleef 2008-08-19 06:08:55 UTC
thaytan, does resindvd fix up p-a-r in this way?
Comment 17 Jan Schmidt 2008-08-19 07:05:30 UTC
Yep, it does - in the rsnparsetter element that sits after the video decoder.
Comment 18 David Schleef 2008-08-19 17:22:42 UTC
OK, then this is fixed.