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 584599 - encoders should forward pixel-aspect-ratio in output caps
encoders should forward pixel-aspect-ratio in output caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
0.10.x
Other Linux
: Normal normal
: 1.0.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-06-02 11:20 UTC by Christian Fredrik Kalager Schaller
Modified: 2012-12-17 11:18 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Christian Fredrik Kalager Schaller 2009-06-02 11:20:36 UTC
When testing creating files for my cellphone I noticed that GStreamer generated files always get the DAR value wrong for files with non-square pixels. Testing with ffenc_mpeg4 it seems qtmux is never told the PAR and thus assumes DAR is equal to pixel height and pixel width. 

My phone complains about corrupt file and refuse to play it and this DAR value is one of the last items different from the working file that ffmpeg creates.
The GStreamer created DAR is 1.22, but since the PAR is 12/11 it should be 4:3
Comment 1 Jan Schmidt 2009-06-02 11:58:56 UTC
It seems sensible that the encoder should copy the pixel-aspect-ratio to the output caps, at the least when it's different to 1:1. That would be complementary behaviour to decoding - where the container demuxer provides the pixel-aspect-ratio, which overrides the pixel-aspect-ratio encoded into the video, when the pixel-aspect-ratio from the container is different to 1:1
Comment 2 Tim-Philipp Müller 2010-02-18 02:08:20 UTC
You should probably clone this bug at least once for every plugin module that needs fixing, if not for every encoder plugin, if you ever want this to go anywhere :)
Comment 3 Tim-Philipp Müller 2010-10-08 18:09:07 UTC
This is filed against core, which is not particularly useful. Moving to -base.
Comment 4 Christian Fredrik Kalager Schaller 2011-05-20 09:45:27 UTC
ok, changing this to ffenc_mpeg4 bug, will create new bugs as I test for other plugins.
Comment 5 Edward Hervey 2011-05-20 11:23:26 UTC
All video encoders should do that, and they should also do it in the other direction (i.e. if upstream calls getcaps, we should forward downstream PAR if present)
Comment 6 Edward Hervey 2011-05-31 12:04:50 UTC
No, really, *ALL* encoders. Don't change the subject of this bug Christian.
Comment 7 Sebastian Dröge (slomo) 2011-06-01 06:36:29 UTC
And this also applies to audio encoders, not only video encoders (number of channels, sampling rate, ...)

Also see bug #651564 and the same probably applies to encoders in good/bad/ugly too.
Comment 8 Mark Nauwelaerts 2011-06-01 08:02:16 UTC
IIRC the base audio encoder class in #642690 caters for such caps proxying, and doing something similar for base video encoder should be quite possible.
Comment 9 Christian Fredrik Kalager Schaller 2011-10-06 09:23:11 UTC
With most elements now ported to the new baseclasses should this bug be marked as solved?
Comment 10 Tim-Philipp Müller 2011-10-10 17:17:20 UTC
> With most elements now ported to the new baseclasses should this bug be marked
> as solved?

It's currently assigned to gst-ffmpeg, so unless this is implemented in gst-ffmpeg encoders already, this bug should probably stay open.
Comment 11 Tim-Philipp Müller 2012-12-14 00:03:25 UTC
This seems to work fine for me now:

 gst-launch-1.0 videotestsrc num-buffers=1 ! video/x-raw,pixel-aspect-ratio=2/1 ! avenc_mpeg4 ! qtmux ! filesink location=/tmp/v.qt

 gst-launch-1.0 playbin uri=file:///tmp/v.qt -v | grep Xv
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstAutoVideoSink:videosink/GstXvImageSink:videosink-actual-sink-xvimage.GstPad:sink: caps = video/x-raw, format=(string)I420, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)2/1, interlace-mode=(string)progressive, colorimetry=(string)bt601, framerate=(fraction)30/1

And both audio and video encoders and decoders now use the base classes, so hopefully handle this correctly.

I think this can be close now, or is anything missing?
Comment 12 Sebastian Dröge (slomo) 2012-12-17 09:24:23 UTC
This should be fine now, the base classes handle all this.