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 609252 - [theoradec] Doesn't handle unknown pixel aspect ratio properly
[theoradec] Doesn't handle unknown pixel aspect ratio properly
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.26
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 609249 609788 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-02-07 16:48 UTC by bens
Modified: 2010-02-13 12:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to fix handling of 0:x PAR (958 bytes, patch)
2010-02-07 16:48 UTC, bens
committed Details | Review

Description bens 2010-02-07 16:48:19 UTC
Created attachment 153209 [details] [review]
patch to fix handling of 0:x PAR

gsttheoradec.c currently says of the pixel aspect ratio

"""
   * 0:0 is allowed and can be interpreted as 1:1, so correct for it.
   * x:0 for other x isn't technically allowed, but it's seen in the wild and
   * is reasonable to treat the same
"""

0:x is treated as 0/x = 0.  This is not correct.  The Theora spec says

"""
If either of these fields are zero, this indicates
that pixel aspect ratio information was not available to the encoder. In
this case it MAY be specified by the application via an external means,
or a default value of 1 : 1 MAY be used.
"""

With the current behavior, I believe a file with PAR 0:x fails to play, and produces errors like

** (totem:14347): CRITICAL **: gst_video_calculate_display_ratio: assertion `num > 0' failed

Attached is a patch to follow the spec.
Comment 1 Sebastian Dröge (slomo) 2010-02-08 07:54:17 UTC
Comment on attachment 153209 [details] [review]
patch to fix handling of 0:x PAR

The patch looks good. Tim, can we get this into 0.10.26? Looks simple enough
Comment 2 Sebastian Dröge (slomo) 2010-02-08 09:12:24 UTC
*** Bug 609249 has been marked as a duplicate of this bug. ***
Comment 3 Tim-Philipp Müller 2010-02-08 12:19:11 UTC
Thanks for the patch. It would be great if you could next time submit a patch created with 'git format-patch -1' - that way I don't have to type in a commit message and the author information :)

 commit a09d9fdece3983a79e4d5a1ccb1ef55a21e158e1
 Author: Benjamin M. Schwartz <bens@alum.mit.edu>
 Date:   Mon Feb 8 11:21:35 2010 +0100

    theoradec: PARs of 0:x, x:0 and 0:0 are all allowed and map to 1:1
    
    Fixes #609252.
Comment 4 Tim-Philipp Müller 2010-02-13 12:35:08 UTC
*** Bug 609788 has been marked as a duplicate of this bug. ***