This video has display_extension which is giving a display_horizatal_size of 720 and display_vertical_size of 576. The PAR is calculating in 1.0 based on this instead of the default width of 352 and height of 576 .
(default widhth/height : valued parsed/calculated from SequenceHdr and SequenceHdrExtension)

But in 0.10 there is no parsing for display_extension and which will use the default width of 352 instead of 720 for PAR calculation.

It seems that the mpeg2dec is giving the PAR 16/15 in both 0.10 and 1.0.(pipeline with out parser element) . But avdec_mpeg2video/ffdec_mpeg2video is giving 24/11 (with out parser element).

If i understood correctly, this is not a bug in 1.0 but instead a "missing feature" in 0.10.

> If i understood correctly, this is not a bug in 1.0 but instead a "missing
> feature" in 0.10.

Sorry, could you explain that remark? Are you saying it is displayed correctly in 1.0?

I think i explained it in comment 1. This video has a display_extension which is not parsing by the 0.10 parser element. Because there was no API for that.
Right now the 1.0 parser is parsing seq_display_ext and calculating correct PAR using gst_mpeg_video_finalise_mpeg2_sequence_header().

Am I missing something ? :)

Let me ask differently:

a) does this play with correct dimensions for you using
    .. ! mpegvideoparse ! {mpeg2dec | avdec_mpeg2video } ! xvimagesink ?
   (it does not for me, the video is about twice as high as it is wide;
    it plays fine with mpegpsdemux ! avdec_mpeg2video ! xvimagesink)

b) do you think it is correct that it is twice as high as it is wide?

In 0.10
---------------
case1: with mpegvideoparse, 

gst-launch -v filesrc location=/home/sreerenj/samples/mpeg2/vlc-record-2011-01-29-22h40m45s-VTS_01_2.VOB-.mpg  ! mpegpsdemux ! mpegvideoparse ! queue ! mpeg2dec ! xvimagesink 

will give the PAR of 24/11.  because of wrong value from mpegvideoparse.(as i explained in comment 1)


case2: with out mpegvideoparse,

gst-launch -v filesrc location=/home/sreerenj/samples/mpeg2/vlc-record-2011-01-29-22h40m45s-VTS_01_2.VOB-.mpg  ! mpegpsdemux !  queue ! mpeg2dec ! xvimagesink

will give par of 16/15 and video seems to be playing as half. I think mpeg2dec is correctly parsing the disply_extension here. And the same result is getting in 1.0 with mpegvideoparse.

What i am saying it that , the PAR calculation seems right in 1.0.

ASFAIK, the display_horizontal_size/display_verical_size for disply_extension is only using for PAR calculation  at the moment.

We might also need to pass it to the downstream element as a video_cropping parameter....

I don't really care too much about what we did in 0.10, but vlc and mplayer play this clip with a 4:3 aspect ratio, and it looks like that's how it's supposed to be played. Surely that's not just coincidence? I want GStreamer to play the clip with a 4:3 aspect ratio as well. So it seems to me like either there's something we're missing, or we're doing the right thing but the data in the extension is bogus, and we should figure out somehow that it's bogus. The video *decoders* should ignore any aspect ratio information they find in the bitstream if upstream (demuxer and/or parser) provide one, so I believe it should be mpegvideoparse's job in this case to figure out the right thing.

Yup, vlc is playing with almost double size  :)..Is it the case which needs frame-cropping support? Do we support frame cropping?,,,which means, if the encoded data specify the cropping region? 

As per the spec , "display_horizontal_size and display_vertical_size together define a rectangle which may be considered as the "intended display's" active region. If this rectangle is smaller than the encoded frame size, then the
display process may be expected to display only a portion of the encoded frame. Conversely if the display rectangle is larger than the encoded frame size, then the display process may be expected to display the reconstructed frames on a
portion of the display device rather than on the whole display device. "

"display_horizontal_size and display_vertical_size do not affect the decoding process but may be used by the displayprocess that is not standardised in this Specification.
"
We are not passing the display_horizontal_size to downstream elements. Which means,

here width=352, height=576, display_horizontal_size=720.
PAR is calculating based on "720" . But video-sink always using the default width "352".

Adding VideoCropMeta to mpeg2dec with display_ext->width and display_ext->height may be a fix for this.(don't know whether it is going to fix the issue...).Or is there some way to communicate the display region to the renderer??

We should just do what DVD players do, as this is outside the MPEG-2 spec.  I'm guessing that when display_horizontal_size > width, this means we should stretch the video to match the display size.  In other words, ignore the display_horizontal_size and calculate the PAR based on the video size.

For display_horizontal_size < width, I am not sure.  It would be best to check pan-scan content to make sure it works.

Created attachment 236734 [details] [review]
 mpeg2dec: make the aspect ratio calculation more  accurate.

This is not a fix for the current issue. Just an enhancement to mpeg2dec.

Created attachment 236735 [details] [review]
configure: bump required libmpeg2 to 0.5.0 (july  12,2008)

Created attachment 246003 [details] [review]
mpeg2dec: make the aspect ratio calculation more accurate.

Merged the previous two patches with slight changes. Instead of bumping the libmpeg2 version , use version check in gstmpeg2dec.c.

commit 866f60cce6ec61407b3b2f5e9dc42610da9bba61
Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com>
Date:   Tue Jun 4 16:12:27 2013 +0300

    mpeg2dec: make the aspect ratio calculation more accurate.
    
    Utilize the libmpeg2 api: mpeg2_guess_aspect() to guess the
    aspect-ratio if we don't have a valid upstream PAR.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=685103

The patch is not a fix the issue mentioned in this bug but just an enhancement to the mpeg2dec.

I think what David said here makes sense and is easy enough to implement

Created attachment 246074 [details] [review]
mpegvideoparser: Fix the pixel-aspect-ratio calculation.

Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com>
Date:   Wed Jun 5 16:16:36 2013 +0300

    mpegvideoparser: Fix the pixel-aspect-ratio calculation
    
    Ignore the display_extension values if they are greater than the width/heigh
    values provided by seqhdr and calculate the PAR based on the seqhdr values.T
    his is what DVD players are doing.
    
    Thanks to "David Schleef <ds@schleef.org>"
    
    https://bugzilla.gnome.org/show_bug.cgi?id=685103

Also picked into 1.0 branch.