Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Video decoder detects wrong pixel aspect ratio #507

Open
reufer opened this issue Nov 30, 2015 · 16 comments
Open

Video decoder detects wrong pixel aspect ratio #507

reufer opened this issue Nov 30, 2015 · 16 comments

Comments

@reufer
Copy link

reufer commented Nov 30, 2015

I have some SD-MPEG2 streams with 16.9 aspect ratio. VLC plays them fine and ffmpeg reports correct dimensions:

Stream #0:0(eng): Video: mpeg2video (Main), yuv420p(tv), 720x576 [SAR 64:45 DAR 16:9], max. 8500 kb/s, 25 fps, 25 tbr, 1k tbn, 50 tbc (default)

However, omxplayer detects a curious pixel aspect ratio of 64/27 and thus shrinks the video vertically:

V:PortSettingsChanged: [email protected] interlace:3 deinterlace:1 anaglyph:0 par:2.37 layer:0 alpha:255

Do you have an explanation for this? I've uploaded a sample here: www.reufer.ch/temp/aspect.mkv

@popcornmix
Copy link
Contributor

I think this is an omxplayer issue, rather than firmware.
Kodi plays the file correctly (in dvdplayer or omxplayer mode).

Mediainfo says:

Display aspect ratio                     : 16:9
Original display aspect ratio            : 2.40:1

Not quite sure what it means by that, but I suspect that the two DARs is the reason for the discrepancy. I'll see if I can trace what information kodi is using, compared to the information omxplayer is using.

popcornmix added a commit to popcornmix/omxplayer that referenced this issue Dec 3, 2015
@popcornmix
Copy link
Contributor

Can you try omxplayer with new commit: popcornmix/omxplayer@89c4b16

@reufer
Copy link
Author

reufer commented Dec 4, 2015

I'm not sure if this is the right way. First of all, my application uses plain TS, so there's no matroska header to check. Second, the sample I provided was originally plain TS, converted to mkv by ffmpeg for a quick test with omxplayer - so the correct parameter must have been inside TS already. In my view, the MPEG2 decoder just reports wrong aspect ratio.

@popcornmix
Copy link
Contributor

Can you provide the ts file?

@reufer
Copy link
Author

reufer commented Dec 4, 2015

Sure: www.reufer.ch/temp/aspect.zip
You can either extract the archive in VDR's video directory as descried in #463 (comment) or look for 00001.ts inside the folder %Stromberg/2004-10-11.20.15.1-1.rec.

@popcornmix
Copy link
Contributor

This will probably need our codec expert (@deborah-c) to comment.

Parsing seq->seq.aspect_ratio_information gives 3 which is a DAR of 16:9.
The code then does:

         // The values we have are DAR, we need to convert to SAR
         if (seq->have_seq_display_ext)
         {
            aspect_y *= seq->seq_display_ext.display_horizontal_size;
            aspect_x *= seq->seq_display_ext.display_vertical_size;
         }
         else
         {
            aspect_y *= seq->vd3_seq.crop_width;
            aspect_x *= seq->vd3_seq.crop_height;
         }

Using seq->vd3_seq.crop_width/seq->vd3_seq.crop_height does give the desired result (seq->vd3_seq.crop_width=720 seq->vd3_seq.crop_height=576).

But seq->seq_display_ext.display_horizontal_size = 540 and seq>seq_display_ext.display_vertical_size = 576. The 540 value is suspicious from the stream. Possibly an encoder error, or something I don't understand.

However it seems wrong to be using that here. We know the shape we want to the frame to be (16:9). We know how many pixels we will send to host (720x576). Host is going to scale the 720x576 pixels by PAR hoping to get 16:9 out.

Using display_horizontal_size (which the host doesn't know about) to determine PAR seems wrong to me.

@reufer
Copy link
Author

reufer commented Dec 4, 2015

Thanks for the detailed analysis. I just checked ffmpeg's parser and display extension is not used at all. Possibly ffmpeg is just too lazy to be fooled by an erroneous stream?

@popcornmix
Copy link
Contributor

If you want to test my proposed firmware change, try:
https://dl.dropboxusercontent.com/u/3669512/temp/firmware_aspect.zip
No guarantee it won't cause regressions, but I'd be interested if it fixes your current problem, and if you can spot any problems.

@reufer
Copy link
Author

reufer commented Dec 6, 2015

Looks good - thanks a lot! I haven't seen any side effects so far, but I'll keep testing.

popcornmix added a commit that referenced this issue Dec 8, 2015
…ck structures used

See: #496

firmware: audioplus: Remove spurious semi-colon that breaks test mode

firmware: vdec3: mpeg2: Don't use display_ext info for calculating PAR
See: #507

firmware: Initial support for setting voltage based on ring osc
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this issue Dec 8, 2015
…ck structures used

See: raspberrypi/firmware#496

firmware: audioplus: Remove spurious semi-colon that breaks test mode

firmware: vdec3: mpeg2: Don't use display_ext info for calculating PAR
See: raspberrypi/firmware#507

firmware: Initial support for setting voltage based on ring osc
@popcornmix
Copy link
Contributor

I have added this to official firmware and will keep an eye out for any bug reports...

@popcornmix
Copy link
Contributor

Okay to close this issue?

@reufer
Copy link
Author

reufer commented Jan 17, 2016

Sure, I haven't seen any side effects. Thanks a lot!

@reufer reufer closed this as completed Jan 17, 2016
@reufer
Copy link
Author

reufer commented Mar 18, 2016

I'm afraid I found a stream where your solution does not work:
http://www.reufer.ch/temp/heitere.ts

The decoder reports pixel aspect ratio of 16/15, however it's a 16:9 video. I'd appreciate if you could have a look at it. Thanks a lot!

@reufer reufer reopened this Mar 18, 2016
neuschaefer pushed a commit to neuschaefer/raspi-binary-firmware that referenced this issue Feb 27, 2017
…ck structures used

See: raspberrypi#496

firmware: audioplus: Remove spurious semi-colon that breaks test mode

firmware: vdec3: mpeg2: Don't use display_ext info for calculating PAR
See: raspberrypi#507

firmware: Initial support for setting voltage based on ring osc
@JamesH65
Copy link
Contributor

@reufer Is this still an issue? Was it a faulty stream?

@reufer
Copy link
Author

reufer commented Dec 21, 2017

I haven't tested recently, but you'll find the stream some lines above. I don't think the stream is faulty, as far as I remember, ffmpeg reported correct dimensions.

@deborah-c
Copy link

deborah-c commented Dec 21, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants