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 350761 - [a52dec] Select output format based on peer element caps
[a52dec] Select output format based on peer element caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
git master
Other Linux
: Normal enhancement
: 0.10.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 169278 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2006-08-10 15:42 UTC by Michal Benes
Modified: 2006-09-08 14:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to control a52dec plugin downmixing (6.75 KB, patch)
2006-08-10 15:44 UTC, Michal Benes
none Details | Review
Patch to control a52dec plugin downmixing (5.95 KB, patch)
2006-08-17 14:40 UTC, Michal Benes
committed Details | Review

Description Michal Benes 2006-08-10 15:42:57 UTC
This patch will select output format (number of channels) based on user preferences (set by properties) or peer element capabilities.

The rationale for this patch is that liba52 can do better downmixing based on a52 hinting than any standalone audioconvert plugin.

Without this patch, 5.1 audio (DVD source) converted to 2-channel (by audioconvert) sounded audible more silent than it should be.
Comment 1 Michal Benes 2006-08-10 15:44:33 UTC
Created attachment 70655 [details] [review]
Patch to control a52dec plugin downmixing
Comment 2 Tim-Philipp Müller 2006-08-10 17:37:50 UTC
> Without this patch, 5.1 audio (DVD source) converted to 2-channel (by
> audioconvert) sounded audible more silent than it should be.

That might actually be a bug, see bug #348002 (just on a side note).
Comment 3 Michael Smith 2006-08-11 11:01:16 UTC
This patch doesn't look sufficient to me.

It adds:
 - a way to explicitly specify the downmix mode (which is fine, but applications aren't going to use that; they'll just use playbin, etc.)
 - checking to see if the peer pad has a fixed number of channels set.

You need to do full caps negotiation instead, so that this _automatically_ works through audioconvert.

This used to be impossible - audioconvert would always choose to do the conversion (I implemented this in fluendo's ac3 decoder, and it didn't work).

However, after I rewrote audioconvert's caps negotiation to advertise that it prefers certain formats above others (it prefers to NOT do channel downmixing, for instance), it should be possible to do this properly.

The caps-nego stuff is pretty complex, though, so I'd expect that to be a bit of  a pain. Some time when I have a chance (i.e. NOT in the next week), I'll probably try implementing this in fluac3dec again. If that works, I'll see if I can provide a patch for a52dec too.
Comment 4 Michal Benes 2006-08-11 12:13:49 UTC
Thank you Michael, I still do not undertand the caps negotiation as good as I'd like to. And I simply didn't dare to do implement the full caps nego.

In fact, in my app, I allways have capsfilter behind a52dec, so this patch worked.
Comment 5 Michal Benes 2006-08-17 14:40:56 UTC
Created attachment 71086 [details] [review]
Patch to control a52dec plugin downmixing

This updated patch automatically chooses prefered number of channels from the source pad caps. With this patch a52dec chooses optimal number of channels even if audioconvert is plugged after a52dec.

Tested with pipes
a52dec ! audioconvert ! alsasink
a52dec ! audioconvert ! audio/x-raw-int,channels=2 ! alsasink
a52dec ! audioconvert ! audio/x-raw-int,channels=1 ! alsasink
Comment 6 Michael Smith 2006-09-01 15:30:15 UTC
Patch looks pretty nice.

Committed with some minor cleanups. Thanks! Sorry it took a while for me to look at this. 
Comment 7 Michael Smith 2006-09-01 15:43:50 UTC
Oops, forgot to close it.
Comment 8 Tim-Philipp Müller 2006-09-08 14:52:27 UTC
*** Bug 169278 has been marked as a duplicate of this bug. ***