Bug 118765 - libvisual receives pcm data only for the left channel
Summary: libvisual receives pcm data only for the left channel
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 1.3.7
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 117598 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-12-21 05:34 UTC by Matt Zukowski
Modified: 2006-06-11 12:32 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Zukowski 2005-12-21 05:34:34 UTC
Version:           1.3.7 (using KDE KDE 3.4.2)
Installed from:    Gentoo Packages
Compiler:          gcc i686-pc-linux-gnu-3.4.4 
OS:                Linux

libvisual is receiving only mono data (i.e. only the left channel is being visualized)

i can't tell if the problem is in vis/libvisual/libvisual.cpp, or somewhere further up

however, i did notice that in libvisual.cpp:304, we have this:

for( uint i = 0; i < 512; i++ )
{
     audio->plugpcm[0][i] = pcm_data[i];
     audio->plugpcm[1][i] = pcm_data[i];
}

which looks to me like the same data is being put into both the left and rich channel buffers. However, this doesn't explain why only left channel data is actually being filled. My guess is something else is going on here, but I'm not a C programmer, and digging any further is hurting my feeble brain.
Comment 1 Matt Zukowski 2006-01-07 18:06:13 UTC
upon further investigation, it looks like the source of the problem seems to be in the way the sound engines generate their scopes.

The gstreamer engine code for example explicitly generates only mono data. At line 406 in gstengine.cpp we have:

for ( long i = 0; i < SCOPE_VALUES; i++, data += channels ) {
    long temp = 0;

    for ( int chan = 0; chan < channels; chan++ ) {
        // Add all channels together so we effectively get a mono scope
        temp += data[offset / sizeof( gint16 ) + chan];
    }
    m_scope[i] = temp / channels;
}

Fixing this will likely affect a lot of things further downstream (the built-in analyzers for example). However, I think this is something that ought to be done, since most visualisation plugins are based on the expecation that stereo data is available (this is why libvisual has seperate data for each channel!).

I've been trying to fix this myself, but I will need some guidance as to what should be done about affected components. For example, what to do about the built in analyzers? Can m_data simply be extended to 1024 bytes to accomodate a second channel, or must the number of scope values be cut to 256, etc.?
Comment 2 Mark Kretschmann 2006-01-08 11:58:47 UTC
Hmm yes, I guess we need to do something about this. Back when we designed the system, we had only the built-in analyzers in mind, which don't really need stereo data.

Shouldn't be too hard to change though.. let's give it some thought.
Comment 3 Matt Zukowski 2006-03-01 08:01:54 UTC
Any updates? I'd have a go at this myself but I'm not sure how to go about it while maintaining compatibility with the existing visualisation code further downstream. Any pointers?
Comment 4 Alexandre Oliveira 2006-03-01 08:03:34 UTC
*** Bug 117598 has been marked as a duplicate of this bug. ***
Comment 5 Alexandre Oliveira 2006-03-01 08:05:10 UTC
AFAIK, nothing changed.
Comment 6 Paul Cifarelli 2006-03-25 05:53:30 UTC
SVN commit 522316 by cifarelli:

Support stereo visualizations for libvisual and xmms.  Built in amaroK
analyzers are still mono, but are converted outside of the engine now

NOTE TO ENGINE WRITERS:
the you now return interleaved pcm to the scope - no longer convert to mono

Here I've updated the xine, helix, and gst10 engines, 
but not 
NMM, KDE MM, akode, arts or gst 0.8

BUG: 118765

 


 M  +7 -1      analyzers/analyzerbase.cpp  
 M  +1 -1      engine/enginebase.cpp  
 M  +1 -0      engine/enginebase.h  
 M  +9 -5      engine/gst10/gstengine.cpp  
 M  +1 -1      engine/gst10/gstengine.h  
 M  +8 -12     engine/helix/helix-engine.cpp  
 M  +1 -1      engine/helix/helix-engine.h  
 M  +2 -4      engine/xine/xine-engine.cpp  
 M  +4 -3      vis/libvisual/libvisual.cpp  
 M  +1 -1      vis/libvisual/libvisual.h  
 M  +9 -2      vis/xmmswrapper/xmmswrapper.cpp  
Comment 7 Matt Zukowski 2006-03-25 18:48:19 UTC
woohoo!