[libavg-devel] Audio Support Patch

[Ulrich von Zadow]

Nick Hebner wrote:
>     To avoid legal issues, I have to ask you to assign the copyright to me:
> 
>     //  Copyright (C) 2007 Ulrich von Zadow
> 
>     If the copyrights are owned by lots of different people, even a
>     technical license change (such as updating to LGPL 3.0) becomes a major
>     hassle because I have to ask everyone who contributed. Of course, I can
>     never change the license of the current version, so there is really
>     nothing to fear. (I'm also pretty committed to open source, but you are
>     under no obligation to believe me ;-).)
> 
>     You can add a line at the bottom of the copyright blurb if you want,
>     though:
> 
>     //  Original author of this file is Nick Hebner (hebern at gmail.com
>     <mailto:hebern at gmail.com>).
> 
> 
> Hmm, ok. I am honestly not all that interested in the legal mumbo jumbo
> involved in licensing, I would just like to have my work attributed to
> me. If this is the easiest way to do that, then that is absolutely fine
> with me.

Thanks :-).

>     > Index: /home/nick/workspace/libavg/src/player/avgdtd.cpp
>     > ===================================================================
>     > --- /home/nick/workspace/libavg/src/player/avgdtd.cpp (revision 2460)
>     > +++ /home/nick/workspace/libavg/src/player/avgdtd.cpp (working copy)
>     > @@ -56,6 +56,8 @@
>     >  "   onkeydown CDATA #IMPLIED\n"
>     >  "   onkeyup CDATA #IMPLIED\n"
>     >  "   enablecrop CDATA #IMPLIED\n"
>     > +"   samplerate CDATA #IMPLIED\n"
>     > +"   channels CDATA #IMPLIED\n"
> 
>     I'm not sure, but shouldn't samplerate and channels be in avgrc instead?
> 
> 
> Can you explain how avgrc is used? A quick look at it seems to indicate
> that these parameters should go there too, but I would like to know how
> these integrated. Looks to me like this just defines variables that
> remain fairly static.

See https://www.libavg.de/wiki/index.php/The_avgrc_File :-).

The code that parses this (I think) contains some completely dead parts
and there's no dtd, but this isn't really complicated. Basically, the
ConfigMgr constructor fills in the allowed fields and loads the files.
It can then be queried for option values.

>     > +// TODO: do we want these to be dynamically changable?
>     > +//void Player::setAudioFrequency(int freq)
>     > +//{
>     > +//    m_AP.m_SampleRate = freq;
>     > +//}
>     > +//
>     > +//void Player::setAudioChannels(int channels)
>     > +//{
>     > +//    m_AP.m_Channels = channels;
>     > +//}
> 
>     Nice to have so tests can run with different frequency/channel setups
> 
> 
> Sure, but making them dynamic would require a restart of the audio
> system. This would mean that we would need to add any locking to make
> that happen, and support dynamic resampling context changes in the video
> decoder. That may or may not be that big of a deal, but I cannot
> envisage these being used for any reason besides testing. As mentioned
> earlier since these are pretty static, they belong in avgrc.

Good point. These functions only need to be available when the player is
not running. Similar to Player::setOGLOptions().

>     Can't m_pAudioEngine->addSource be called from inside the Node? In
>     Video::setDisplayEngine() perhaps? Would probably be a lot less code and
>     get rid of the dynamic casts.
> 
> Would we want to add a Node::setAudioEngine() that is overridden by
> video and audio nodes? Seems to make more sense semantically then adding
> this to setDisplayEngine(). Alternatively, we could make
> setDisplayEngine a more general setRenderingEngines(DisplayEngine,
> AudioEngine) or something like that.

I think I like setRenderingEngines() most, since these functions will be
called one after the other anyway.

Cheers,

  Uli

-- 

Ulrich von Zadow | +49-172-7872715
Jabber: cocacoder at jabber.berlin.ccc.de
Skype: uzadow