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 675451 - a52dec: can update caps if original number of audio channels has changed
a52dec: can update caps if original number of audio channels has changed
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
0.10.x
Other All
: Normal normal
: 0.10.20
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-05-04 15:20 UTC by Julien Isorce
Modified: 2012-05-17 11:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
can update caps if original number of channels has changed (898 bytes, patch)
2012-05-04 15:20 UTC, Julien Isorce
none Details | Review

Description Julien Isorce 2012-05-04 15:20:11 UTC
Created attachment 213451 [details] [review]
can update caps if original number of channels has changed

If the number of channels is changing, then a52dec should give a chance to retrieve the orignal channels.

For exemple, if there are 2 audio channels and some minutes later, there are 6 audio channels in the ac3 packets, then the actual a52dec downmix from 6 to 2  audio channels.

The patch allows to update caps if original number of channels has changed.
Comment 1 Wim Taymans 2012-05-09 13:51:25 UTC
The patch looks wrong, it compares flags against using_channels, it should probably only compare the flags related to the channels. the code around there is also a bit hard to follow.
Comment 2 Julien Isorce 2012-05-09 13:59:43 UTC
(In reply to comment #1)
> The patch looks wrong, it compares flags against using_channels, it should
> probably only compare the flags related to the channels. the code around there
> is also a bit hard to follow.

In the original code, some lines later there is:

flags = a52dec->using_channels;

and also:

channels = flags & (A52_CHANNEL_MASK | A52_LFE);
...
a52dec->using_channels = channels;



So should I use:

if (a52dec->using_channels != flags & (A52_CHANNEL_MASK | A52_LFE)) {

instead of:

if (a52dec->using_channels != flags) {

?
Comment 3 Mark Nauwelaerts 2012-05-17 11:18:22 UTC
In addition to the mask the test should also not be against using_channels (which specifies the current output), but rather stream_channels (which tracks the input stream channels).  The original patch would repeatedly renegotiate if e.g. downstream could only handle 2 channels (using_channels) of the 6 input channels (stream_channels).

[0.10]
commit 98b8c1ce564705ea9370d9cb453f4075a407a03b
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Thu May 17 12:40:05 2012 +0200

    a52dec: trigger renegotiation upon changed stream channels
    
    Fixes #675451

[0.11]
commit 4d97760993ddef99175f1dc2bcf1444fcd1943f9
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Thu May 17 12:40:05 2012 +0200

    a52dec: trigger renegotiation upon changed stream channels
    
    Fixes #675451