Some general remarks...
On Wed, Apr 01, 2009 at 01:20:14AM +0200, Dale Glass wrote:
> +
> + LLMediaBase * getStreamMedia() { return mInternetStreamMedia; }
Are we not following const correctness?
Normally this should be:
LLMediaBase* getStreamMedia() const { return mInternetStreamMedia; }
> public:
> F32 mMaxWindGain; // Hack. Public to set before fade in?
>
> diff --git a/indra/llmedia/llmediaimplgstreamer.cpp
> b/indra/llmedia/llmediaimplgstreamer.cpp
> index 51614c5..d4e8fc2 100644
> --- a/indra/llmedia/llmediaimplgstreamer.cpp
> +++ b/indra/llmedia/llmediaimplgstreamer.cpp
> @@ -73,7 +73,8 @@ LLMediaImplGStreamer () :
> mTextureFormatType ( LL_MEDIA_UNSIGNED_INT_8_8_8_8_REV ),
> mPump ( NULL ),
> mPlaybin ( NULL ),
> - mVideoSink ( NULL )
> + mVideoSink ( NULL ),
> + mLastTitle ( "" )
A std::string is empty by default, there is no need to list
it in the initialization list like this.
> #ifdef LL_GST_SOUNDSINK
> ,mAudioSink ( NULL )
> #endif // LL_GST_SOUNDSINK
> @@ -300,6 +301,8 @@ LLMediaImplGStreamer::bus_callback (GstBus *bus,
> case GST_STATE_PAUSED:
> break;
> case GST_STATE_PLAYING:
> + impl->mLastTitle = "";
> +
impl->mLastTitle.clear(); is faster.
> + case GST_MESSAGE_TAG:
> + {
> + DEBUGMSG("GST message tag");
> + GstTagList *new_tags = NULL;
> +
> + gst_message_parse_tag( message, &new_tags );
> +
> + if ( new_tags )
> + {
> + gchar *title = NULL;
> + if ( gst_tag_list_get_string(new_tags, GST_TAG_TITLE,
> &title) )
> + {
> + gst_tag_list_free(new_tags);
> + std::string newtitle(title);
> +
> + if ( newtitle != impl->mLastTitle && newtitle
> != "" )
... && !newtitle.empty()
is faster, in fact I'd turn this around:
if (!newtitle.empty() && newtitle != impl->mLastTitle)
> + {
> + impl->mLastTitle = newtitle;
> + LLMediaEvent event( impl,
> impl->mLastTitle );
> + impl->getEventEmitter().update(
> &LLMediaObserver::onMediaTitleChange, event );
> + }
> +
> + g_free(title);
I'd move this up, to directly below the std::string newtitle(title)
> +class LLTitleObserver
> + : public LLMediaObserver
> +{
> +public:
> + void init(std::string url);
Pass by value?? This isn't java or LSL :p
Surely you meant:
void init(std::string const& url);
> + /*virtual*/ void onMediaTitleChange(const EventType& event_in);
> +private:
> + LLMediaBase* mMediaSource;
> +};
> +
> +static LLTitleObserver sTitleObserver;
> +
> +static LLRegisterWidget<LLMediaRemoteCtrl> r("media_remote");
static is not used anymore in C++. You'd normally an anonymous
namespace. More importantly however, global variables are the
root of all evil. I'm worried when I see things like this.
--
Carlo Wood <[email protected]>
_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/SLDev
Please read the policies before posting to keep unmoderated posting privileges
